Re: [PATCH v2 0/8] Improve ld.lld-21+ compatibility when building i386-pc target

2026-02-12 Thread Nicholas Vinson

On 2/12/26 02:53, Radoslav Kolev wrote:

On Wed, Feb 11, 2026 at 1:28 AM Nicholas Vinson 
The changes for ofpath.c are a bit more involved because the original
code would modify the string 'ed' pointed to. With C23, ed needs to be a
'const char *' because sysfs_path is a 'const char *'. This also means
that the line "*q = '\0'" is no longer valid because you cannot safely
modify a const char string.


I may be completely missing something important here, but I run into
the same issue trying to build against glibc 2.43 and my fix for
ofpath.c was less invasive. I'm sending my patch in a reply, please
have a look.


I think the issue is I missed the xstrdup() call on my first pass 
through. That said, on re-evaluation, I find the xstrdup() call to be 
superfluous and recommend removing it. It's removal would allow the 
function to execute without reduced dynamic memory allocation (only 
space for path is needed) and reduces the total number of strstr() calls 
to 1.


I want to review my commit messages before publishing the revised patch 
(which I'll do later).


Thanks,
Nicholas Vinson



Regards,
Rado



___
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 0/8] Improve ld.lld-21+ compatibility when building i386-pc target

2026-02-11 Thread Radoslav Kolev via Grub-devel
On Wed, Feb 11, 2026 at 1:28 AM Nicholas Vinson  The changes for ofpath.c are a bit more involved because the original
> code would modify the string 'ed' pointed to. With C23, ed needs to be a
> 'const char *' because sysfs_path is a 'const char *'. This also means
> that the line "*q = '\0'" is no longer valid because you cannot safely
> modify a const char string.

I may be completely missing something important here, but I run into
the same issue trying to build against glibc 2.43 and my fix for
ofpath.c was less invasive. I'm sending my patch in a reply, please
have a look.

Regards,
Rado

___
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 0/8] Improve ld.lld-21+ compatibility when building i386-pc target

2026-02-11 Thread Daniel Kiper
On Tue, Feb 10, 2026 at 04:47:41PM +0100, Daniel Kiper wrote:
> On Mon, Feb 09, 2026 at 07:43:28PM -0500, Nicholas Vinson wrote:
> > Starting with ld.llvm-21, any attempt create a non-relocatable binary and 
> > set
> > one more secton addresses below 0x40 results in a linker error. 
> > Furthermore,
> > the differences between ld.bfd and ld.lld made finding a proper set of
> > command-line flags tht worked with both linkers and bypass the image base
> > address restriction difficult. Therefore, the approach of using a custom 
> > linker
> > script was adopted to solve the issue.
> >
> > This approach was tested using:
> >
> > ../configure CC=clang CXX=clang++ LDFLAGS="-fuse-ld=lld" 
> > TARGET_LDFLAGS="-fuse-ld=lld" --with-platform=pc
> > ../configure CC=clang CXX=clang++ --with-platform=pc (both with ld.lld as 
> > the default and ld.bfd as the default)
> > ../configure CC=gcc CXX=g++ --with-platform=pc
> >
> > and a VM was used for testing. To build the disk images the VM was booted 
> > with,
> > the following scripts were used:
>
> [...]
>
> > In all cases, the VM successfully booted to the standard GRUB prompt.
> >
> > Nicholas Vinson (8):
> >   i386/pc/int.h: conditionally apply regparm attr.
> >   grub-core: Update kernel image generation
> >   i386-cygwin-img-ld.sc -> i386-cygwin-img.lds
> >   Revert "configure: Print a more helpful error if autoconf-archive is
> > not installed"
> >   Revert "configure: Check linker for --image-base support"
> >   Revert "INSTALL: Add note that the GNU Autoconf Archive may be needed"
> >   configure: drop -Ttext checks for i386-pc
>
> For all these patches Reviewed-by: Daniel Kiper ...

Sadly your patch set breaks Windows build... :-(

  $ ./configure --build=x86_64-linux-gnu --host=i686-w64-mingw32 
--target=i686-w64-mingw32 --with-platform=pc

  $ make

  [...]

  i686-w64-mingw32-gcc -std=gnu99 -fno-common -Os -m32 -Wall -W -Wshadow 
-Wpointer-arith -Wundef -Wchar-subscripts -Wcomment -Wdeprecated-declarations 
-Wdisabled-optimization -Wdiv-by-zero -Wfloat-equal -Wformat-extra-args 
-Wformat-security -Wformat-y2k -Wimplicit -Wimplicit-function-declaration 
-Wimplicit-int -Wmain -Wmissing-braces -Wmissing-format-attribute -Wmultichar 
-Wparentheses -Wreturn-type -Wsequence-point -Wshadow -Wsign-compare -Wswitch 
-Wtrigraphs -Wunknown-pragmas -Wunused -Wunused-function -Wunused-label 
-Wunused-parameter -Wunused-value  -Wunused-variable -Wwrite-strings 
-Wnested-externs -Wstrict-prototypes -g -Wredundant-decls -Wmissing-prototypes 
-Wmissing-declarations  -Wextra -Wattributes -Wendif-labels -Winit-self 
-Wint-to-pointer-cast -Winvalid-pch -Wmissing-field-initializers -Wnonnull 
-Woverflow -Wvla -Wpointer-to-int-cast -Wstrict-aliasing -Wvariadic-macros 
-Wvolatile-register-var -Wpointer-sign -Wmissing-include-dirs 
-Wmissing-prototypes -Wmissing-declarations -Wformat=2 -march=i386 -mrtd 
-mregparm=3 -falign-functions=1 -falign-loops=1 -falign-jumps=1 
-freg-struct-return -mno-mmx -mno-sse -mno-sse2 -mno-sse3 -mno-3dnow 
-msoft-float -fno-omit-frame-pointer -fno-dwarf2-cfi-asm -fno-reorder-functions 
-mno-stack-arg-probe -fno-asynchronous-unwind-tables -fno-unwind-tables 
-fno-ident -fno-stack-protector -Wtrampolines -Werror   -DGRUB_HAS_PCI 
-fno-builtin   -m32 -Wl,-mi386pe  -nostdlib -Wl,-N -Wl,-S 
-Wl,-T../conf/i386-cygwin-img.lds -Wl,-T../conf/i386-pc-kernel.lds 
-Wl,-Ttext=0x8200   -o lzma_decompress.image.exe 
boot/i386/pc/lzma_decompress_image-startup_raw.o
  /usr/bin/i686-w64-mingw32-ld:../conf/i386-pc-kernel.lds:19: undefined symbol 
`_grub_text_base' referenced in expression
  /usr/bin/i686-w64-mingw32-ld:../conf/i386-pc-kernel.lds:19: undefined symbol 
`_grub_text_base' referenced in expression
  collect2: error: ld returned 1 exit status
  collect2: error: ld returned 1 exit status
  /usr/bin/i686-w64-mingw32-ld:../conf/i386-pc-kernel.lds:19: undefined symbol 
`_grub_text_base' referenced in expression
  make[3]: *** [Makefile:33123: boot.image.exe] Error 1
  make[3]: *** Waiting for unfinished jobs
  make[3]: *** [Makefile:33145: boot_hybrid.image.exe] Error 1
  collect2: error: ld returned 1 exit status
  /usr/bin/i686-w64-mingw32-ld:../conf/i386-pc-kernel.lds:19: undefined symbol 
`_grub_text_base' referenced in expression
  collect2: error: ld returned 1 exit status
  make[3]: *** [Makefile:33276: cdboot.image.exe] Error 1
  make[3]: *** [Makefile:36161: pxeboot.image.exe] Error 1
  /usr/bin/i686-w64-mingw32-ld:../conf/i386-pc-kernel.lds:19: undefined symbol 
`_grub_text_base' referenced in expression
  /usr/bin/i686-w64-mingw32-ld:../conf/i386-pc-kernel.lds:19: undefined symbol 
`_grub_text_base' referenced in expression
  collect2: error: ld returned 1 exit status
  collect2: error: ld returned 1 exit status
  /usr/bin/i686-w64-mingw32-ld:../conf/i386-pc-kernel.lds:19: undefined symbol 
`_grub_text_base' referenced in expression
  collect2: error: ld returned 1 exit status
  make[3]: *** [Makefile:35303: lnxboot.image.exe] Error 1
  m

Re: [PATCH v2 0/8] Improve ld.lld-21+ compatibility when building i386-pc target

2026-02-11 Thread Daniel Kiper
On Tue, Feb 10, 2026 at 06:27:49PM -0500, Nicholas Vinson wrote:
> On 2/10/26 10:47, Daniel Kiper wrote:
> > On Mon, Feb 09, 2026 at 07:43:28PM -0500, Nicholas Vinson wrote:
> > > Starting with ld.llvm-21, any attempt create a non-relocatable binary and 
> > > set
> > > one more secton addresses below 0x40 results in a linker error. 
> > > Furthermore,
> > > the differences between ld.bfd and ld.lld made finding a proper set of
> > > command-line flags tht worked with both linkers and bypass the image base
> > > address restriction difficult. Therefore, the approach of using a custom 
> > > linker
> > > script was adopted to solve the issue.
> > >
> > > This approach was tested using:
> > >
> > > ../configure CC=clang CXX=clang++ LDFLAGS="-fuse-ld=lld" 
> > > TARGET_LDFLAGS="-fuse-ld=lld" --with-platform=pc
> > > ../configure CC=clang CXX=clang++ --with-platform=pc (both with ld.lld as 
> > > the default and ld.bfd as the default)
> > > ../configure CC=gcc CXX=g++ --with-platform=pc
> > >
> > > and a VM was used for testing. To build the disk images the VM was booted 
> > > with,
> > > the following scripts were used:
> >
> > [...]
> >
> > > In all cases, the VM successfully booted to the standard GRUB prompt.
> > >
> > > Nicholas Vinson (8):
> > >i386/pc/int.h: conditionally apply regparm attr.
> > >grub-core: Update kernel image generation
> > >i386-cygwin-img-ld.sc -> i386-cygwin-img.lds
> > >Revert "configure: Print a more helpful error if autoconf-archive is
> > >  not installed"
> > >Revert "configure: Check linker for --image-base support"
> > >Revert "INSTALL: Add note that the GNU Autoconf Archive may be needed"
> > >configure: drop -Ttext checks for i386-pc
> >
> > For all these patches Reviewed-by: Daniel Kiper ...
> >
> > >C23 fixes: fix strchr() and strrchr() handling
> >
> > It seems to me that this patch does more than commit message says.
> > I think it has to be split into more parts or commit message has to be
> > improved.
>
> I admit the message is a bit terse, but I kept the changes to a minimum. I
> did, however, forget to mention strstr().

If you mention strstr() I am OK with it...

> Put simply, with C23, if the first argument to strstr(), strchr(), or
> strrchar() is a const char *, the result is a const char *. C23 also made
> this change to about 9 or so other functions.
>
> As a result, most of the changes were just changing a variable from 'char *'
> to 'const char *'.
>
> The changes for ofpath.c are a bit more involved because the original code
> would modify the string 'ed' pointed to. With C23, ed needs to be a 'const
> char *' because sysfs_path is a 'const char *'. This also means that the
> line "*q = '\0'" is no longer valid because you cannot safely modify a const
> char string.
>
> I saw two options to fix this, the first was to modify the line to be q =
> strndup(...);, add the requisite error checking, and then make sure to free
> q just before every subsequent return. The second option, is what I did,
> find the difference between q and ed, save it off, and then replace the
> strlen(ed) calls with that difference.
>
> I believe the second approach is the better of the two. It did have the
> ancillary effect of updating a few snprintf() calls and the path_size
> calculation; however, these changes are required with this approach.

I am OK with second approach but I think this change should go to
separate patch, with an explanation like above, because it is more
complicated than just adding const.

> If you'd prefer, I can re-submit patch 8 with these details in the commit
> message. Alternatively, if you would prefer the strndup() approach, I will
> respin this patch series without patch 8, and then submit a re-worked C23
> patch as a separate request.

Just split patch #8 as I suggested above and repost C23 as separate series.

Daniel

___
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 0/8] Improve ld.lld-21+ compatibility when building i386-pc target

2026-02-10 Thread Nicholas Vinson

On 2/10/26 10:47, Daniel Kiper wrote:

On Mon, Feb 09, 2026 at 07:43:28PM -0500, Nicholas Vinson wrote:

Starting with ld.llvm-21, any attempt create a non-relocatable binary and set
one more secton addresses below 0x40 results in a linker error. Furthermore,
the differences between ld.bfd and ld.lld made finding a proper set of
command-line flags tht worked with both linkers and bypass the image base
address restriction difficult. Therefore, the approach of using a custom linker
script was adopted to solve the issue.

This approach was tested using:

../configure CC=clang CXX=clang++ LDFLAGS="-fuse-ld=lld" 
TARGET_LDFLAGS="-fuse-ld=lld" --with-platform=pc
../configure CC=clang CXX=clang++ --with-platform=pc (both with ld.lld as the 
default and ld.bfd as the default)
../configure CC=gcc CXX=g++ --with-platform=pc

and a VM was used for testing. To build the disk images the VM was booted with,
the following scripts were used:


[...]


In all cases, the VM successfully booted to the standard GRUB prompt.

Nicholas Vinson (8):
   i386/pc/int.h: conditionally apply regparm attr.
   grub-core: Update kernel image generation
   i386-cygwin-img-ld.sc -> i386-cygwin-img.lds
   Revert "configure: Print a more helpful error if autoconf-archive is
 not installed"
   Revert "configure: Check linker for --image-base support"
   Revert "INSTALL: Add note that the GNU Autoconf Archive may be needed"
   configure: drop -Ttext checks for i386-pc


For all these patches Reviewed-by: Daniel Kiper ...


   C23 fixes: fix strchr() and strrchr() handling


It seems to me that this patch does more than commit message says.
I think it has to be split into more parts or commit message has to be
improved.


I admit the message is a bit terse, but I kept the changes to a minimum. 
I did, however, forget to mention strstr().


Put simply, with C23, if the first argument to strstr(), strchr(), or 
strrchar() is a const char *, the result is a const char *. C23 also 
made this change to about 9 or so other functions.


As a result, most of the changes were just changing a variable from 
'char *' to 'const char *'.


The changes for ofpath.c are a bit more involved because the original 
code would modify the string 'ed' pointed to. With C23, ed needs to be a 
'const char *' because sysfs_path is a 'const char *'. This also means 
that the line "*q = '\0'" is no longer valid because you cannot safely 
modify a const char string.


I saw two options to fix this, the first was to modify the line to be q 
= strndup(...);, add the requisite error checking, and then make sure to 
free q just before every subsequent return. The second option, is what I 
did, find the difference between q and ed, save it off, and then replace 
the strlen(ed) calls with that difference.


I believe the second approach is the better of the two. It did have the 
ancillary effect of updating a few snprintf() calls and the path_size 
calculation; however, these changes are required with this approach.


If you'd prefer, I can re-submit patch 8 with these details in the 
commit message. Alternatively, if you would prefer the strndup() 
approach, I will respin this patch series without patch 8, and then 
submit a re-worked C23 patch as a separate request.


Thanks,
Nicholas Vinson



Daniel



___
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 0/8] Improve ld.lld-21+ compatibility when building i386-pc target

2026-02-10 Thread Daniel Kiper
On Mon, Feb 09, 2026 at 07:43:28PM -0500, Nicholas Vinson wrote:
> Starting with ld.llvm-21, any attempt create a non-relocatable binary and set
> one more secton addresses below 0x40 results in a linker error. 
> Furthermore,
> the differences between ld.bfd and ld.lld made finding a proper set of
> command-line flags tht worked with both linkers and bypass the image base
> address restriction difficult. Therefore, the approach of using a custom 
> linker
> script was adopted to solve the issue.
>
> This approach was tested using:
>
> ../configure CC=clang CXX=clang++ LDFLAGS="-fuse-ld=lld" 
> TARGET_LDFLAGS="-fuse-ld=lld" --with-platform=pc
> ../configure CC=clang CXX=clang++ --with-platform=pc (both with ld.lld as the 
> default and ld.bfd as the default)
> ../configure CC=gcc CXX=g++ --with-platform=pc
>
> and a VM was used for testing. To build the disk images the VM was booted 
> with,
> the following scripts were used:

[...]

> In all cases, the VM successfully booted to the standard GRUB prompt.
>
> Nicholas Vinson (8):
>   i386/pc/int.h: conditionally apply regparm attr.
>   grub-core: Update kernel image generation
>   i386-cygwin-img-ld.sc -> i386-cygwin-img.lds
>   Revert "configure: Print a more helpful error if autoconf-archive is
> not installed"
>   Revert "configure: Check linker for --image-base support"
>   Revert "INSTALL: Add note that the GNU Autoconf Archive may be needed"
>   configure: drop -Ttext checks for i386-pc

For all these patches Reviewed-by: Daniel Kiper ...

>   C23 fixes: fix strchr() and strrchr() handling

It seems to me that this patch does more than commit message says.
I think it has to be split into more parts or commit message has to be
improved.

Daniel

___
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel