Re: ld/emultempl/elf32.em trusts inode numbers even under Mingwin

Re: ld/emultempl/elf32.em trusts inode numbers even under Mingwin

[Zack Weinberg]

"Dave Korn" <dave.korn@artimi.com> writes:

>> > It looks to me that mingw sets the st_dev field to the index of the
>> > DOS drive letter (excluding any skipped devices, so on my PC, A=1,
>> > C=2, etc...) and sets the st_ino to zero.
>> 
>> Okay.  That's mostly consistent with my observations, though I was
>> working out of Y: and I don't know why st_dev was zero... but there's
>> plenty weird about my Windows test box, so it could just be me.
>
>   Yeh.  What kind of drive is your Y:?  SMB or NFS remote, perhaps?
> That might make a difference.  Anyway, it seems like testing inode
> == 0 should be effective.  Or just disable the optimisation
> altogether for mingw.  I don't think there's any straightforward,
> quick, efficient way to tell if two files are the same in the
> general case.

It was indeed NFS remote.  I agree, testing inode == 0 ought to be
adequate.  Here's a revised patch.

zw

        * emultempl/elf32.em (gld${EMULATION_NAME}_stat_needed):
        Do not indicate a duplicate if st_ino is zero.

===================================================================
Index: ld/emultempl/elf32.em
--- ld/emultempl/elf32.em	3 Feb 2005 14:12:54 -0000	1.127
+++ ld/emultempl/elf32.em	8 Feb 2005 20:14:58 -0000
@@ -231,8 +231,16 @@ gld${EMULATION_NAME}_stat_needed (lang_i
       return;
     }
 
+  /* Some operating systems, e.g. Windows, do not provide a meaningful
+     st_ino; they always set it to zero.  (Windows _does_ provide a
+     meaningful st_dev.)  Do not indicate a duplicate library in that
+     case.  While there is no guarantee that a system that provides
+     meaningful inode numbers will never set st_ino to zero, that
+     inode number is reserved by all Unix file systems in common use
+     (UFS, ext2, etc.) so we need not worry about that possibility.  */
   if (st.st_dev == global_stat.st_dev
-      && st.st_ino == global_stat.st_ino)
+      && st.st_ino == global_stat.st_ino
+      && st.st_ino != 0)
     {
       global_found = TRUE;
       return;

RE: ld/emultempl/elf32.em trusts inode numbers even under Mingwin

[Dave Korn]

> -----Original Message-----
> From: Zack Weinberg 
> Sent: 08 February 2005 20:19

> It was indeed NFS remote.  I agree, testing inode == 0 ought to be
> adequate.  Here's a revised patch.

  Looks good to me, although I'm not a mingw expert.  The only tiny nit I might
pick would perhaps be to extend the comment slightly by explicitly mentioning
that even if a bogus match on st_inode == 0 ever did occur, it would only be a
missed optimisation rather than actually cause any harm; that's only an
implication at the moment.

    cheers, 
      DaveK
-- 
Can't think of a witty .sigline today....

Re: ld/emultempl/elf32.em trusts inode numbers even under Mingwin

[Zack Weinberg]

"Dave Korn" <dave.korn@artimi.com> writes:

>> -----Original Message-----
>> From: Zack Weinberg 
>> Sent: 08 February 2005 20:19
>
>> It was indeed NFS remote.  I agree, testing inode == 0 ought to be
>> adequate.  Here's a revised patch.
>
>   Looks good to me, although I'm not a mingw expert.  The only tiny
> nit I might pick would perhaps be to extend the comment slightly by
> explicitly mentioning that even if a bogus match on st_inode == 0
> ever did occur, it would only be a missed optimisation rather than
> actually cause any harm; that's only an implication at the moment.

Er - is it just a missed optimization?  I couldn't tell from the rest
of the code.  Loading the same shared library twice *sounds* like
trouble...

zw

Re: ld/emultempl/elf32.em trusts inode numbers even under Mingwin

[Alan Modra]

On Wed, Feb 09, 2005 at 10:26:50AM -0800, Zack Weinberg wrote:
> of the code.  Loading the same shared library twice *sounds* like
> trouble...

It isn't.  See elflink.c

      ret = elf_add_dt_needed_tag (abfd, info, soname, add_needed);
      if (ret < 0)
	goto error_return;

      /* If we have already included this dynamic object in the
	 link, just ignore it.  There is no reason to include a
	 particular dynamic object more than once.  */
      if (ret > 0)
	return TRUE;

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: ld/emultempl/elf32.em trusts inode numbers even under Mingwin

[Zack Weinberg]

[-- Attachment #1: Type: text/plain, Size: 311 bytes --]

On Thu, 2005-02-10 at 08:18 +1030, Alan Modra wrote:
>       /* If we have already included this dynamic object in the
> 	 link, just ignore it.  There is no reason to include a
> 	 particular dynamic object more than once.  */
>       if (ret > 0)
> 	return TRUE;

Great, so, how about this revised patch?

zw

[-- Attachment #2: diff.txt --]
[-- Type: text/plain, Size: 955 bytes --]

===================================================================
Index: ld/emultempl/elf32.em
--- ld/emultempl/elf32.em	3 Feb 2005 14:12:54 -0000	1.127
+++ ld/emultempl/elf32.em	11 Feb 2005 20:00:30 -0000
@@ -231,8 +231,16 @@ gld${EMULATION_NAME}_stat_needed (lang_i
       return;
     }
 
+  /* Some operating systems, e.g. Windows, do not provide a meaningful
+     st_ino; they always set it to zero.  (Windows does provide a
+     meaningful st_dev.)  Do not indicate a duplicate library in that
+     case.  While there is no guarantee that a system that provides
+     meaningful inode numbers will never set st_ino to zero, this is
+     merely an optimization, so we do not need to worry about false
+     negatives.  */
   if (st.st_dev == global_stat.st_dev
-      && st.st_ino == global_stat.st_ino)
+      && st.st_ino == global_stat.st_ino
+      && st.st_ino != 0)
     {
       global_found = TRUE;
       return;

Re: ld/emultempl/elf32.em trusts inode numbers even under Mingwin

[Alan Modra]

On Fri, Feb 11, 2005 at 12:07:31PM -0800, Zack Weinberg wrote:
> On Thu, 2005-02-10 at 08:18 +1030, Alan Modra wrote:
> >       /* If we have already included this dynamic object in the
> > 	 link, just ignore it.  There is no reason to include a
> > 	 particular dynamic object more than once.  */
> >       if (ret > 0)
> > 	return TRUE;
> 
> Great, so, how about this revised patch?

Yes, this is OK to apply.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: ld/emultempl/elf32.em trusts inode numbers even under Mingwin

[Zack Weinberg]

"Dave Korn" <dave.korn@artimi.com> writes:

>
>   It wasn't clear from your initial post what system was the target
> here: you mentioned the cygming host, which can target either
> i686-pc-cygwin or i686-pc-mingw, and of course which 'stat' function
> you get depends on which one of those you choose.
>
>   Assuming your customers are compiling under a cygming host, but
> using the "-mno-cygwin" switch to target mingw, the change to ||
> from && should do it. 

Umm.  I think we have terminology issues... let me be extremely
specific.  I build toolchains with --build=i686-linux,
--host=i686-mingw32, --target=$CPU-vxworks (there are about six
choices for target CPU).  I give the resulting binaries to the
customers, who use them to compile code for their embedded targets.

Cygwin is not involved at any stage of the process, nor does anyone
other than me generate binaries which run under Windows.

> It looks to me that mingw sets the st_dev field to the index of the
> DOS drive letter (excluding any skipped devices, so on my PC, A=1,
> C=2, etc...) and sets the st_ino to zero.

Okay.  That's mostly consistent with my observations, though I was
working out of Y: and I don't know why st_dev was zero... but there's
plenty weird about my Windows test box, so it could just be me.

zw

Re: ld/emultempl/elf32.em trusts inode numbers even under Mingwin

[Zack Weinberg]

Zack Weinberg <zack@codesourcery.com> writes:

> "Dave Korn" <dave.korn@artimi.com> writes:
>>   I must throw in an AYS? here.... AFAIK, cygwin generates pseudo
>> inode-nums by hashing the filename.  This behaviour is inconsistent
>> across different windows versions (NT vs 9x series), different
>> filesystems (FAT vs NTFS particularly), and
>> local-vs-remote-SMB-mounted drives.  In a quick test, I couldn't
>> even get that result when using the -mno-cygwin flag to build a
>> mingw-targeted rather than cygwin-targeted; the inode was zero, but
>> the device field wasn't.
>
> All I can say is that I observe all four values as zero in my
> relatively limited testing.  I build my Windows-hosted toolchains
> with cross compilers from i686-linux to i686-mingw32, though.
>
> Rereading, I see I got the test wrong - it was meant to be 
> (st.st_dev != 0 || st.st_ino != 0).  I'm not quite sure I understand
> what you are saying; does that correction address your concern?

It's been pointed out to me that MSVCRT's documentation claims to
provide meaningful st_dev (it corresponds to the drive letter).  I
don't know why I was getting zeroes then.  Anyway, at this point I
throw up my hands and plead for help from someone who knows more about
Windows.  (Maybe we should #if that block out if (_WIN32 &&
!__CYGWIN__) rather than being clever?)

zw