ld: don't use SAME_INODE for the duplicate-script check on hosts without inodes
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_binutils_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_binutils_check--master-arm |
success
|
Test passed
|
Commit Message
Since 2.47, ld rejects a perfectly ordinary link on native Windows:
ld.exe: error: linker script file '../common_arm/ldscript.common
(ldscript-flash)' appears multiple times
when the only thing on the command line is a single -T, and that script
INCLUDEs one other file. The two names in the message are the giveaway:
the file being opened and an entry already recorded are different files,
so the comparison that matched them is wrong.
Two changes stack up to produce it. d048eee29108 ("ld: Use stat to check
if linker script appears multiple times") changed the PR 24576 check from
a name comparison to stat plus SAME_INODE. Then 47071f8b14a0
("same-inode.h: don't depend on _GL_WINDOWS_STAT_INODES") dropped the
guard in include/same-inode.h that had been expanding SAME_INODE to a
literal 0 on native Windows. binutils never defines
_GL_WINDOWS_STAT_INODES, so on Windows the check went from dead code to
live in one release.
The Windows CRT sets st_ino to 0 for every file. The guard that survived
only rejects st_ino == 0 && st_dev == 0, and st_dev is the drive number,
so on D: it is 3 and the guard passes. Every file on the drive then
compares equal to every other file, and the first INCLUDE inside a -T
script looks like a repeat of the script itself. The commit message of
47071f8b14a0 anticipates this: "this doesn't really make SAME_INODE
usable on windows hosts as a number of the likely filesystems (FAT,
HPFS, or NTFS) don't support st_ino."
Fall back to comparing file names when stat gives no usable inode, so the
duplicate detection keeps working on hosts where inodes are real and stops
firing on files that merely share a device. PR 24576's own testcases still
pass, including the ././/script spelling that a name comparison alone would
miss, because hosts with real inodes still take the inode path.
Signed-off-by: Cole Munz <Munzzyy1@proton.me>
---
Notes on how far I got testing this, since I could not test on Windows.
I reproduced the failure on Linux by interposing stat/lstat/fstat to return
what the Windows CRT returns (st_ino = 0, st_dev = 3) and running the real
link command from the project that hit this. Stock 2.47 under that shim gives
the byte-identical error; with this patch it links. On normal POSIX stat the
same build still rejects -T script -T script, and still rejects the
-T ././/script -T script spelling, which is the case a plain name comparison
would miss.
Not covered: I have no mingw-w64 host, so this was never compiled for or run
on native Windows, and I have no dejagnu here so I could not run the real
ld testsuite. The PR 24576 cases above were replayed by hand from
ld/testsuite/ld-scripts/pr24576-1.d and -2.d rather than run under the
harness.
For what it is worth on impact: MSYS2 currently ships 2.47 as
arm-none-eabi-binutils in ucrt64, and it breaks the bootrom link for every
user of the proxmark3 project's Windows environment, not just their CI.
Comments
On Mon, Aug 17, 2026 at 04:59:00PM +0000, Cole Munz wrote:
> The Windows CRT sets st_ino to 0 for every file. The guard that survived
> only rejects st_ino == 0 && st_dev == 0, and st_dev is the drive number,
> so on D: it is 3 and the guard passes. Every file on the drive then
> compares equal to every other file
That says there is a bug in SAME_INODE, and in upstream gnulib too.
I will apply the following.
* same-inode.h (SAME_INODE): Do not test st_dev on windows to
validate st_dev/st_ino comparison.
diff --git a/include/same-inode.h b/include/same-inode.h
index 9d9049843d1..d9aea7b006f 100644
--- a/include/same-inode.h
+++ b/include/same-inode.h
@@ -28,11 +28,12 @@
&& (a).st_dev == (b).st_dev)
# elif defined _WIN32 && ! defined __CYGWIN__
/* Native Windows. */
- /* stat() and fstat() set st_dev and st_ino to 0 if information about
+ /* stat() and fstat() set st_ino to 0 if information about
the inode is not available. */
# define SAME_INODE(a, b) \
- (!((a).st_ino == 0 && (a).st_dev == 0) \
- && (a).st_ino == (b).st_ino && (a).st_dev == (b).st_dev)
+ ((a).st_ino != 0 \
+ && (a).st_ino == (b).st_ino \
+ && (a).st_dev == (b).st_dev)
# else
# define SAME_INODE(a, b) \
((a).st_ino == (b).st_ino \
On Tue, Aug 18, 2026 at 03:30:44PM +0930, Alan Modra wrote:
> That says there is a bug in SAME_INODE, and in upstream gnulib too.
> I will apply the following.
Agreed, and that is the right layer for it. I tested the two macros side by
side:
two different files on D: (st_ino 0, st_dev 3)
old -> 1 treats them as the same file
new -> 0 correct
two different files on drive 0 (st_ino 0, st_dev 0)
old -> 0 new -> 0
same file, real inode info
old -> 1 new -> 1 still detects real duplicates
different files, real inode info
old -> 0 new -> 0
So it removes the false positive without weakening the case where inodes
work.
One consequence worth a decision. Once SAME_INODE is false whenever st_ino
is 0, ld's PR 24576 check goes inert on native Windows, so someone who does
pass the same script twice there gets no diagnostic at all. My patch kept
that check alive on such hosts by falling back to a name comparison when
stat gives nothing usable.
Either is defensible. Drop my ld patch and let PR 24576 only cover hosts
with real inodes, or take same-inode.h plus the name fallback and keep the
diagnostic everywhere. I have no preference, I just did not want the check
to go quiet as a side effect rather than as a choice.
Cole
On 18.08.2026 08:00, Alan Modra wrote:
[...]
> That says there is a bug in SAME_INODE, and in upstream gnulib too.
> I will apply the following.
IMO, it would also be nice to mark pr24576-1 and pr24576-2 as XFAIL
rather than simply skipping them. Perhaps duplicate detection on Windows
will be fixed at some point in the future.
/J.D.
ld/testsuite/ld-scripts/pr24576-1.d | 3 +++
ld/testsuite/ld-scripts/pr24576-2.d | 3 +++
ld/testsuite/ld-scripts/script.exp | 8 ++------
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/ld/testsuite/ld-scripts/pr24576-1.d b/ld/testsuite/ld-scripts/pr24576-1.d
index 6cc7621aadb..892a12ed4e3 100644
--- a/ld/testsuite/ld-scripts/pr24576-1.d
+++ b/ld/testsuite/ld-scripts/pr24576-1.d
@@ -1,3 +1,6 @@
#source: default-script.s
#ld: -defsym _START=0x800 -T default-script.t -T default-script.t
#error: .*default-script.t\)' appears multiple times
+#
+# XFAIL the test case on MinGW/Windows hosts due to broken SAME_INODE macro.
+#xfail: [ishost\ *-mingw*]
diff --git a/ld/testsuite/ld-scripts/pr24576-2.d b/ld/testsuite/ld-scripts/pr24576-2.d
index 2d26ab32783..eba4770e228 100644
--- a/ld/testsuite/ld-scripts/pr24576-2.d
+++ b/ld/testsuite/ld-scripts/pr24576-2.d
@@ -1,3 +1,6 @@
#source: default-script.s
#ld: -defsym _START=0x800 -T ././/default-script.t -T default-script.t
#error: .*default-script.t\)' appears multiple times
+#
+# XFAIL the test case on MinGW/Windows hosts due to broken SAME_INODE macro.
+#xfail: [ishost\ *-mingw*]
diff --git a/ld/testsuite/ld-scripts/script.exp b/ld/testsuite/ld-scripts/script.exp
index ff65e397b6b..ed4cbf7660e 100644
--- a/ld/testsuite/ld-scripts/script.exp
+++ b/ld/testsuite/ld-scripts/script.exp
@@ -233,12 +233,8 @@ run_dump_test "pr20302"
run_dump_test "output-section-types"
run_dump_test "ld-version"
run_dump_test "ld-version-2"
-# Windows hosts are likely to have filesystems where st_ino has no meaning,
-# breaking the test for accidental duplicate scripts.
-if { ![ishost *-mingw*] } {
- run_dump_test "pr24576-1"
- run_dump_test "pr24576-2"
-}
+run_dump_test "pr24576-1"
+run_dump_test "pr24576-2"
run_dump_test "linker-script-not-found"
run_dump_test "segment-start" {{name (default)}}
On 18.08.2026 12:48, Jan Dubiec wrote:
> IMO, it would also be nice to mark pr24576-1 and pr24576-2 as XFAIL
> rather than simply skipping them. Perhaps duplicate detection on Windows
> will be fixed at some point in the future.
Agreed that XFAIL records it better than a skip. Worth treating the two
separately though, because they stop failing at different points.
pr24576-1.d: -T default-script.t -T default-script.t
pr24576-2.d: -T ././/default-script.t -T default-script.t
With only the same-inode.h change, SAME_INODE is false for every pair on
Windows, so both fail and both want XFAIL.
With the name fallback as well, -1 compares two identical strings and
passes there. -2 is exactly the spelling a name comparison cannot see,
which is why it was written that way, so it stays XFAIL either way
unless someone canonicalises the path first.
So how many of them need the marker depends on which fix goes in, and
that is Alan's call rather than mine.
Cole
On Tue, Aug 18, 2026 at 06:30:59AM +0000, Cole Munz wrote:
> On Tue, Aug 18, 2026 at 03:30:44PM +0930, Alan Modra wrote:
> > That says there is a bug in SAME_INODE, and in upstream gnulib too.
> > I will apply the following.
>
> Agreed, and that is the right layer for it. I tested the two macros side by
> side:
>
> two different files on D: (st_ino 0, st_dev 3)
> old -> 1 treats them as the same file
> new -> 0 correct
> two different files on drive 0 (st_ino 0, st_dev 0)
> old -> 0 new -> 0
> same file, real inode info
> old -> 1 new -> 1 still detects real duplicates
> different files, real inode info
> old -> 0 new -> 0
>
> So it removes the false positive without weakening the case where inodes
> work.
>
> One consequence worth a decision. Once SAME_INODE is false whenever st_ino
> is 0, ld's PR 24576 check goes inert on native Windows, so someone who does
> pass the same script twice there gets no diagnostic at all.
Right. binutils-2.33 through binutils-2.45 had checks that compared
file names, which worked on windows except when people were creative
with paths. binutils-2.46 had no check on windows and binutile-2.47
was broken by me noticing the check wasn't enabled.
I'll apply your patch.
> My patch kept
> that check alive on such hosts by falling back to a name comparison when
> stat gives nothing usable.
>
> Either is defensible. Drop my ld patch and let PR 24576 only cover hosts
> with real inodes, or take same-inode.h plus the name fallback and keep the
> diagnostic everywhere. I have no preference, I just did not want the check
> to go quiet as a side effect rather than as a choice.
>
> Cole
On Wed, Aug 19, 2026 at 03:44:21PM +0930, Alan Modra wrote:
> Right. binutils-2.33 through binutils-2.45 had checks that compared
> file names, which worked on windows except when people were creative
> with paths. binutils-2.46 had no check on windows and binutile-2.47
> was broken by me noticing the check wasn't enabled.
>
> I'll apply your patch.
I pushed an edited version, because I'm not sure all hosts will avoid
using a zero st_ino for files. Posix doesn't say anything about
st_ino values as far as I could see, just "The st_ino and st_dev
fields taken together uniquely identify the file within the system"
and further wording that says this is true for networked filesystems
too.
diff --git a/ld/ldfile.c b/ld/ldfile.c
index 00fe1d90d44..cad1e168af0 100644
--- a/ld/ldfile.c
+++ b/ld/ldfile.c
@@ -881,15 +881,30 @@ ldfile_find_command_file (const char *name,
the same linker script twice. */
if (stat (filename, &sbuf1) == 0)
{
- struct stat sbuf2;
+#if defined _WIN32 && ! defined __CYGWIN__
+ /* Native Windows stat reports st_ino as zero on most file
+ systems. Compare file names there. */
+ bool have_inode = sbuf1.st_ino != 0;
+#else
+ bool have_inode = true;
+#endif
+
for (script = processed_scripts;
script != NULL;
script = script->next)
- if ((open_how != script_nonT || script->open_how != script_nonT)
- && stat (script->name, &sbuf2) == 0
- && SAME_INODE (sbuf1, sbuf2))
- fatal (_("%P: error: linker script file '%s (%s)'"
- " appears multiple times\n"), filename, script->name);
+ {
+ struct stat sbuf2;
+
+ if (open_how == script_nonT && script->open_how == script_nonT)
+ continue;
+
+ if (have_inode
+ ? (stat (script->name, &sbuf2) == 0
+ && SAME_INODE (sbuf1, sbuf2))
+ : filename_cmp (filename, script->name) == 0)
+ fatal (_("%P: error: linker script file '%s (%s)'"
+ " appears multiple times\n"), filename, script->name);
+ }
}
len = strlen (filename);
On Tue, Aug 18, 2026 at 12:48:05PM +0200, Jan Dubiec wrote:
> On 18.08.2026 08:00, Alan Modra wrote:
> [...]
>
> > That says there is a bug in SAME_INODE, and in upstream gnulib too.
> > I will apply the following.
> IMO, it would also be nice to mark pr24576-1 and pr24576-2 as XFAIL rather
> than simply skipping them. Perhaps duplicate detection on Windows will be
> fixed at some point in the future.
I question whether all filesystems on Windows have st_ino zero.
https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/stat-functions?view=msvc-170
says that st_ino on FAT, HPFS, and NTFS "has no meaning" and from that
we assume st_ino of zero.
Cole's patch, and the one I committed both allow for the possibility
of st_ino being valid on Windows, which means that if you xfail the
tests you'll get XPASS if st_ino is valid. So I think it best to
leave the testsuite as is.
On 22.08.2026 05:58, Alan Modra wrote:
> On Tue, Aug 18, 2026 at 12:48:05PM +0200, Jan Dubiec wrote:
>> On 18.08.2026 08:00, Alan Modra wrote:
>> [...]
>>
>>> That says there is a bug in SAME_INODE, and in upstream gnulib too.
>>> I will apply the following.
>> IMO, it would also be nice to mark pr24576-1 and pr24576-2 as XFAIL rather
>> than simply skipping them. Perhaps duplicate detection on Windows will be
>> fixed at some point in the future.
>
> I question whether all filesystems on Windows have st_ino zero.
> https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/stat-functions?view=msvc-170
> says that st_ino on FAT, HPFS, and NTFS "has no meaning" and from that
> we assume st_ino of zero.
Agreed. BTW. What value does st_ino have on Linux for a file on a FAT
filesystem?
>
> Cole's patch, and the one I committed both allow for the possibility
> of st_ino being valid on Windows, which means that if you xfail the
> tests you'll get XPASS if st_ino is valid. So I think it best to
> leave the testsuite as is.
>
I don't see anything wrong with XFAIL turning into XPASS at some point.
IMO, XFAIL/XPASS convey quite similar information ("the issue here is
known and it's not fatal"), which is better than having no information
at all.
/J.D.
On 22.08.2026 09:08, Jan Dubiec wrote:
> Agreed. BTW. What value does st_ino have on Linux for a file on a FAT
> filesystem?
Linux makes the number up rather than reading it off the disk.
fat_build_inode() sets i_ino = iunique(sb, MSDOS_ROOT_INO) in
fs/fat/inode.c, and iunique() starts its counter above max_reserved and
keeps going until it finds a value not already in use, so you get a
nonzero number and never 0. The comment above that code says it
outright: "i_ino is constant and has nothing with on-disk location".
The root directory is a fixed 1 and FSINFO is 2.
The catch is where the number comes from. fat_build_inode() looks for
an existing inode by on-disk position first and only calls iunique()
when it has to build a new one, so the value is stable while that inode
is alive, but it is not derived from the disk and a remount hands out
fresh ones.
So SAME_INODE does work on Linux over FAT, which is what makes the zero
check a Windows thing rather than a FAT thing. That is read out of the
source though, not measured. I have no FAT filesystem here to stat.
Cole
@@ -882,19 +882,30 @@
success:
/* PR 24576: Catch the case where the user has accidentally included
- the same linker script twice. */
- if (stat (filename, &sbuf1) == 0)
- {
- struct stat sbuf2;
- for (script = processed_scripts;
- script != NULL;
- script = script->next)
- if ((open_how != script_nonT || script->open_how != script_nonT)
- && stat (script->name, &sbuf2) == 0
- && SAME_INODE (sbuf1, sbuf2))
+ the same linker script twice. Not every host has usable inodes:
+ native Windows stat always reports st_ino as zero, which would make
+ every script look like every other one. Compare file names there. */
+ {
+ bool have_inode = stat (filename, &sbuf1) == 0 && sbuf1.st_ino != 0;
+
+ for (script = processed_scripts;
+ script != NULL;
+ script = script->next)
+ {
+ struct stat sbuf2;
+
+ if (open_how == script_nonT && script->open_how == script_nonT)
+ continue;
+
+ if (have_inode
+ ? (stat (script->name, &sbuf2) == 0
+ && sbuf2.st_ino != 0
+ && SAME_INODE (sbuf1, sbuf2))
+ : filename_cmp (filename, script->name) == 0)
fatal (_("%P: error: linker script file '%s (%s)'"
" appears multiple times\n"), filename, script->name);
- }
+ }
+ }
len = strlen (filename);
script = xmalloc (sizeof (*script) + len);