On Thu, Aug 01, 2013 at 04:39:35PM +0300, Konstantin Belousov wrote:
> On Wed, Jul 31, 2013 at 10:12:31PM -0400, Mark Johnston wrote:
> > On Mon, Jul 29, 2013 at 10:54:49PM +0200, Mateusz Guzik wrote:
> > > On Mon, Jul 29, 2013 at 11:56:25AM -0400, Mark Johnston wrote:
> > > > > 127276 suggests running the binary as is (which I don't like) and
> > > > > achieves this with a hacky way. So if we really want to do this, the
> > > > > patch should be reworked to detect Linux binaries properly.
> > > > >
> > > > > In general we should gain linux_ldd (like linux_kdump) and our ldd
> > > > > should work only on FreeBSD binaries. The last part is achieved with
> > > > > my
> > > > > patch.
> > > > >
> > > > > markj, are you working on this?
> > > >
> > > > Not really; my original fix for this problem was essentially the same as
> > > > yours. That is, just change ldd(1) to bail if the OS ABI byte isn't
> > > > equal to ELFOSABI_FREEBSD. That's the change I have committed in my
> > > > local tree right now.
> > > >
> > > > Then I thought I'd try to get ldd to work properly with Linux binaries
> > > > as well, but wasn't sure what the right approach should be. As the above
> > > > PR suggests, the easy thing to do is to just pass
> > > > LD_TRACE_LOADED_OBJECTS and not LD_32_TRACE_LOADED_OBJECTS for 32-bit
> > > > ELF objects if the OS isn't FreeBSD. This feels somewhat hacky to me,
> > > > but I didn't really see another approach.
> > > >
> > > > That said, I think your patch should be committed since it's clearly an
> > > > improvement over the current behaviour. I'm willing to test and commit
> > > > it, and clean up the open PRs. If you could expand on the right way to
> > > > handle Linux binaries, I'd be willing to implement and commit that too.
> > > > I don't quite understand your reference to linux_kdump though - I have
> > > > no such program on my laptop running CURRENT, and ktrace+kdump seem to
> > > > work fine with the Linux binaries under /compat/linux.
> > > >
> > >
> > > Well, there was linux_kdump in ports but it apparently got obsolete as
> > > necessary support for included in our regular kdump.
> > >
> > > So it may make sense to teach our ldd how to deal with Linux binaries
> > > for consistency, but its unclear for me if this is better than providing
> > > linux_ldd. Also there is the problem of (not) appending /compat/linux to
> > > printed paths (for Linux binaries the kernel performs file lookups against
> > > /compat/linux first). I'm not that interested in this problem though. :P
> >
> > What do you think of the patch below, which just sets both variables in
> > the compat case? It's somewhat less intrusive than the patch in the PR.
> > If that's no good then I'll just commit your original patch; I really
> > just want to fix the fact that the example pipeline in the ldd(1) man
> > page starts a GTK program (some Adobe Flash thingy to be specific) when
> > run in /usr/local/bin on my desktop machine. :)
> >
> > [snip]
>
> I do not understand this COMPAT_32BIT business in ldd(1). Its only
> application seems to preventing ldd 32bit binary from amd64 host to
> work on i386 ?
>
> IMO, both LD_TRACE and LD_32_TRACE should be passed unconditionally
> always. I do not see any harm of doing this.
I spent some time trying to figure out if there was any reason for this
and didn't come up with anything. Does the patch below look ok? It just
adds a couple of macros to set both the native and 32-bit compat
variable. It works properly for me for 32-bit, native, and (32-bit)
Linux executables and shared libs on an amd64 machine.
diff --git a/usr.bin/ldd/ldd.c b/usr.bin/ldd/ldd.c
index 00c8797..39f38e8 100644
--- a/usr.bin/ldd/ldd.c
+++ b/usr.bin/ldd/ldd.c
@@ -49,12 +49,6 @@ __FBSDID("$FreeBSD$");
#include "extern.h"
-#ifdef COMPAT_32BIT
-#define LD_ "LD_32_"
-#else
-#define LD_ "LD_"
-#endif
-
/*
* 32-bit ELF data structures can only be used if the system header[s] declare
* them. There is no official macro for determining whether they are declared,
@@ -76,13 +70,23 @@ static void usage(void);
#define _PATH_LDD32 "/usr/bin/ldd32"
+#define LDD_SETENV(name, value, overwrite) do { \
+ setenv("LD_" name, value, overwrite); \
+ setenv("LD_32_" name, value, overwrite); \
+} while (0)
+
+#define LDD_UNSETENV(name) do { \
+ unsetenv("LD_" name); \
+ unsetenv("LD_32_" name); \
+} while (0)
+
static int
execldd32(char *file, char *fmt1, char *fmt2, int aflag, int vflag)
{
char *argv[8];
int i, rval, status;
- unsetenv(LD_ "TRACE_LOADED_OBJECTS");
+ LDD_UNSETENV("TRACE_LOADED_OBJECTS");
rval = 0;
i = 0;
argv[i++] = strdup(_PATH_LDD32);
@@ -121,7 +125,7 @@ execldd32(char *file, char *fmt1, char *fmt2, int aflag,
int vflag)
}
while (i--)
free(argv[i]);
- setenv(LD_ "TRACE_LOADED_OBJECTS", "yes", 1);
+ LDD_SETENV("TRACE_LOADED_OBJECTS", "yes", 1);
return (rval);
}
#endif
@@ -210,15 +214,15 @@ main(int argc, char *argv[])
}
/* ld.so magic */
- setenv(LD_ "TRACE_LOADED_OBJECTS", "yes", 1);
+ LDD_SETENV("TRACE_LOADED_OBJECTS", "yes", 1);
if (fmt1 != NULL)
- setenv(LD_ "TRACE_LOADED_OBJECTS_FMT1", fmt1, 1);
+ LDD_SETENV("TRACE_LOADED_OBJECTS_FMT1", fmt1, 1);
if (fmt2 != NULL)
- setenv(LD_ "TRACE_LOADED_OBJECTS_FMT2", fmt2, 1);
+ LDD_SETENV("TRACE_LOADED_OBJECTS_FMT2", fmt2, 1);
- setenv(LD_ "TRACE_LOADED_OBJECTS_PROGNAME", *argv, 1);
+ LDD_SETENV("TRACE_LOADED_OBJECTS_PROGNAME", *argv, 1);
if (aflag)
- setenv(LD_ "TRACE_LOADED_OBJECTS_ALL", "1", 1);
+ LDD_SETENV("TRACE_LOADED_OBJECTS_ALL", "1", 1);
else if (fmt1 == NULL && fmt2 == NULL)
/* Default formats */
printf("%s:\n", *argv);
_______________________________________________
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
