On Wed, Apr 20, 2005 at 12:12:54PM +0200, Markus F.X.J. Oberhumer wrote:> Misha Brukman wrote: > >On Tue, Apr 19, 2005 at 07:01:40AM +0200, Markus F.X.J. Oberhumer > >wrote: Have you considered using bugpoint for your codegen debugging > >needs? http://llvm.cs.uiuc.edu/docs/Bugpoint.html#codegendebug > > Well, the (critical) bug in question was > http://llvm.cs.uiuc.edu/bugs/show_bug.cgi?id=548 , and I'm not sure if > bugpoint could have helped me reducing the testcase down from a huge > program.Actually, that is the `point' of bugpoint, pun intended. :)> But then, I've never looked too closely - is there a small tutorial ?http://llvm.cs.uiuc.edu/docs/HowToSubmitABug.html About the only thing we haven't gotten to `bugpoint' is front-end miscompilations -- those test cases need to be narrowed down by hand, but once your code is in LLVM bytecode, bugpoint can rip it into shreds.> >For the isExecutable patch, you check to see if it's a file first before > >calling access(), but that precludes that function being used on a > >directory (which is a valid sys::Path object and could be queried for > >being executable), so I think it's unnecessary. > > I'm pretty sure that the semantics of isExecutable() means "can I > excecute a _file_". Without this patch FindExecutable() returns > directories on $PATH - see > http://llvm.cs.uiuc.edu/bugs/show_bug.cgi?id=545 .You and Reid have a point. I'm convinced. Patched, bug closed.> > I also applied your patch to ignore dangling symlinks. > > Fine, so http://llvm.cs.uiuc.edu/bugs/show_bug.cgi?id=543 can be closed.As per Reid's comment, I'll leave it open until this issue is resolved: 3. Figure out why ThrowErrno isn't appending the standard operating system error message that would tell us why the status couldn't be obtained.> >Finally, I applied your patch to ArchiveWriter with minor > >modifications -- please see llvm-commits or the web archive. > > I didn't use get{u,g}id() because that's not portable - I think we > need another abstraction in the System library for these.Hmm, seems like it's available on the Unices that we support: Linux, OS X, and SunOS. Man page lists getgid() as conforming to POSIX and BSD 4.3, so I'm assuming *BSD has them too (can't confirm for lack of a *BSD system). As soon as we run into a Unix system that we want to support, we might have to go the abstraction route, but it seems to work for our currently-supported platforms.> Do you really want external patches for this ? A simple Perl script > that runs on all *.h and *.cpp files, and a local commit from your > side would be much simpler. The testsuite should also be enhanced to > daily report files with trailing whitespace. Please note that this not > purely academic - trailing whitespace are a horror for anyone > maintaining external patches against a CVS tree.I'll look into this. Thanks, -- Misha Brukman :: http://misha.brukman.net :: http://llvm.cs.uiuc.edu
Misha Brukman wrote:>>I didn't use get{u,g}id() because that's not portable - I think we >>need another abstraction in the System library for these. >> >> > >Hmm, seems like it's available on the Unices that we support: Linux, OS >X, and SunOS. Man page lists getgid() as conforming to POSIX and BSD >4.3, so I'm assuming *BSD has them too (can't confirm for lack of a *BSD >system). As soon as we run into a Unix system that we want to support, >we might have to go the abstraction route, but it seems to work for our >currently-supported platforms. > >FreeBSD has them. According to its man pages, getuid was introduced in Version 7 AT&T Unix. Ancient history. It doesn't say when getgid was introduced, but it's just as old as it's documented in my 1980 edition Berkeley VAX UNIX manual, and in any case both are in POSIX.1 and therefore ought to be portable by definition.
On Wed, 2005-04-20 at 10:55 -0500, Misha Brukman wrote:> > > I also applied your patch to ignore dangling symlinks. > > > > Fine, so http://llvm.cs.uiuc.edu/bugs/show_bug.cgi?id=543 can be closed. > > As per Reid's comment, I'll leave it open until this issue is resolved: > > 3. Figure out why ThrowErrno isn't appending the standard operating > system error message that would tell us why the status couldn't be > obtained.I've committed a patch to Path.inc that (hopefully) resolves this situation. I believe the assignment to errno was not working because its a macro on some platforms. In any event, this is hard to test because you have to create error situations in the file system. I'll get there someday but its not a large enough issue to warrant significant time investment. I also cleaned up the error messages so they all have a similar format. Bug 543 is resolved as a consequence. Reid. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20050420/f8ec72a1/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 189 bytes Desc: This is a digitally signed message part URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20050420/f8ec72a1/attachment.sig>
Markus, This patch can't be applied. Not all platforms have getgid() and getuid () functions. Placing non-portable code outside of lib/System is deprecated. We set the values to 1000 by default because in general the uid/gid doesn't matter in an archive and the 1000 value gets you to a safe (non-root, non-system) value. If a specific value for these is needed on a given platform, then we need to implement something like "getDefaultUserId" and "getDefaultGroupId" functions in lib/System and use those in lib/Bytecode/Archive. Is there a specific problem that is driving these changes? Reid. On Thu, 2005-04-21 at 17:17 +0200, Markus F.X.J. Oberhumer wrote:> Jeff Cohen wrote: > > Misha Brukman wrote: > > > >>> I didn't use get{u,g}id() because that's not portable - I think we > >>> need another abstraction in the System library for these. > >>> > >> > >> > >> Hmm, seems like it's available on the Unices that we support: Linux, OS > >> X, and SunOS. Man page lists getgid() as conforming to POSIX and BSD > >> 4.3, so I'm assuming *BSD has them too (can't confirm for lack of a *BSD > >> system). As soon as we run into a Unix system that we want to support, > >> we might have to go the abstraction route, but it seems to work for our > >> currently-supported platforms. > >> > >> > > FreeBSD has them. According to its man pages, getuid was introduced in > > Version 7 AT&T Unix. Ancient history. It doesn't say when getgid was > > introduced, but it's just as old as it's documented in my 1980 edition > > Berkeley VAX UNIX manual, and in any case both are in POSIX.1 and > > therefore ought to be portable by definition. > > Does windows have these ? > > In any case, below is another small patch for lib/Bytecode/Archive. > > ~Markus > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://mail.cs.uiuc.edu/mailman/listinfo/llvmdev_______________________ Reid Spencer President & CTO eXtensible Systems, Inc. rspencer at x10sys.com -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20050421/45ef3b33/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 189 bytes Desc: This is a digitally signed message part URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20050421/45ef3b33/attachment.sig>
Nope. Win32 is not (and does not pretend to be) POSIX compliant. These functions cannot be used outside of lib/System/Unix. New methods need to be added to class llvm::sys::Process. Markus F.X.J. Oberhumer wrote:> Jeff Cohen wrote: > >> Misha Brukman wrote: >> >>>> I didn't use get{u,g}id() because that's not portable - I think we >>>> need another abstraction in the System library for these. >>>> >>> >>> >>> >>> Hmm, seems like it's available on the Unices that we support: Linux, OS >>> X, and SunOS. Man page lists getgid() as conforming to POSIX and BSD >>> 4.3, so I'm assuming *BSD has them too (can't confirm for lack of a >>> *BSD >>> system). As soon as we run into a Unix system that we want to support, >>> we might have to go the abstraction route, but it seems to work for our >>> currently-supported platforms. >>> >>> >> FreeBSD has them. According to its man pages, getuid was introduced >> in Version 7 AT&T Unix. Ancient history. It doesn't say when getgid >> was introduced, but it's just as old as it's documented in my 1980 >> edition Berkeley VAX UNIX manual, and in any case both are in POSIX.1 >> and therefore ought to be portable by definition. > > > Does windows have these ? > > In any case, below is another small patch for lib/Bytecode/Archive. > > ~Markus > >------------------------------------------------------------------------ > >Index: Archive.cpp >==================================================================>RCS file: /var/cvs/llvm/llvm/lib/Bytecode/Archive/Archive.cpp,v >retrieving revision 1.6 >diff -u -r1.6 Archive.cpp >--- Archive.cpp 28 Jan 2005 01:17:07 -0000 1.6 >+++ Archive.cpp 21 Apr 2005 15:13:24 -0000 >@@ -40,8 +40,8 @@ > ArchiveMember::ArchiveMember() > : next(0), prev(0), parent(0), path("<invalid>"), flags(0), data(0) > { >- info.user = 1000; >- info.group = 1000; >+ info.user = getuid(); >+ info.group = getgid(); > info.mode = 0777; > info.fileSize = 0; > info.modTime = sys::TimeValue::now(); > >
On Thu, 2005-04-21 at 17:56 +0200, Markus F.X.J. Oberhumer wrote:> Reid Spencer wrote: > > If a specific value for these is > > needed on a given platform, then we need to implement something like > > "getDefaultUserId" and "getDefaultGroupId" functions in lib/System and > > use those in lib/Bytecode/Archive. > > That's probably the way to go.Implemented as sys::Process::GetCurrentUserId() and sys::Process::GetCurrentGroupId(). Both Unix and Win32 implementations have been provided. Jeff, is there something more intelligent on Win32 that these methods could return (other than 65536)?. I've also corrected the ArchiveWriter to use these methods when it is defaulting the archive header. Reid -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20050421/fe35e8cd/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 189 bytes Desc: This is a digitally signed message part URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20050421/fe35e8cd/attachment.sig>
Nope. Any number is as good as any other. Reid Spencer wrote:> On Thu, 2005-04-21 at 17:56 +0200, Markus F.X.J. Oberhumer wrote: > >>Reid Spencer wrote: >>> If a specific value for these is >>> needed on a given platform, then we need to implement something like >>> "getDefaultUserId" and "getDefaultGroupId" functions in lib/System and >>> use those in lib/Bytecode/Archive. >> >>That's probably the way to go. >> >> > > Implemented as sys::Process::GetCurrentUserId() and > sys::Process::GetCurrentGroupId(). Both Unix and Win32 implementations > have been provided. Jeff, is there something more intelligent on Win32 > that these methods could return (other than 65536)?. > > I've also corrected the ArchiveWriter to use these methods when it is > defaulting the archive header. > > Reid > >------------------------------------------------------------------------ > >_______________________________________________ >LLVM Developers mailing list >LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >http://mail.cs.uiuc.edu/mailman/listinfo/llvmdev > >
On Thu, Apr 21, 2005 at 05:56:43PM +0200, Markus F.X.J. Oberhumer wrote:> Reid Spencer wrote: > >This patch can't be applied. Not all platforms have getgid() and > >getuid() functions. Placing non-portable code outside of lib/System > >is deprecated. We set the values to 1000 by default because in > >general the uid/gid doesn't matter in an archive and the 1000 value > >gets you to a safe (non-root, non-system) value. > > I had thought so, but Misha started using these in ArchiveWriter.cpp.Whoops. Mea culpa. After the lib/System/Unix patches, I was mentally still in Unix-land (hence all my Unix references). Thanks for fixing, Reid! :) -- Misha Brukman :: http://misha.brukman.net :: http://llvm.cs.uiuc.edu
Yeah, that's fine. I'll change it soon. Reid. On Thu, 2005-04-21 at 18:32 +0200, Markus F.X.J. Oberhumer wrote:> Reid Spencer wrote: > > On Thu, 2005-04-21 at 17:56 +0200, Markus F.X.J. Oberhumer wrote: > > > >>Reid Spencer wrote: > >>> If a specific value for these is > >>> needed on a given platform, then we need to implement something like > >>> "getDefaultUserId" and "getDefaultGroupId" functions in lib/System and > >>> use those in lib/Bytecode/Archive. > >> > >>That's probably the way to go. > >> > > > > Implemented as sys::Process::GetCurrentUserId() and > > sys::Process::GetCurrentGroupId(). Both Unix and Win32 implementations > > have been provided. Jeff, is there something more intelligent on Win32 > > that these methods could return (other than 65536)?. > > > > I've also corrected the ArchiveWriter to use these methods when it is > > defaulting the archive header. > > What do you think about this updated patch ? > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://mail.cs.uiuc.edu/mailman/listinfo/llvmdev_______________________ Reid Spencer President & CTO eXtensible Systems, Inc. rspencer at x10sys.com -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20050421/d7804e0d/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 189 bytes Desc: This is a digitally signed message part URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20050421/d7804e0d/attachment.sig>