This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH: Properly dump addend in readelf


On Sat, 2 Oct 2010, H.J. Lu wrote:

> On Sat, Oct 2, 2010 at 5:17 AM, Joseph S. Myers <joseph@codesourcery.com> wrote:
> > On Fri, 1 Oct 2010, H.J. Lu wrote:
> >
> >> ? ? ? ? ? ? ? ? if (off < 0)
> >> - ? ? ? ? ? ? ? ? printf (" - %lx", - off);
> >> + ? ? ? ? ? ? ? ? printf (" - %llx", - off);
> >> ? ? ? ? ? ? ? ? else
> >> - ? ? ? ? ? ? ? ? printf (" + %lx", off);
> >> + ? ? ? ? ? ? ? ? printf (" + %llx", off);
> >
> > Use of %ll formats isn't portable across hosts; MinGW needs %I64. ?See
> > BFD_VMA_FMT in bfd-in.h, for example, or print_vma in binutils/prdbg.c.
> >
> 
> 
> BFD_VMA_FMT doesn't always work with long long. llx has been used:

I was giving examples of how this can be done portably.

> dwarf.c:  snprintf (buff, sizeof (buff), "%16.16llx ", val);

This has a proper __MSVCRT__ conditional and was one of the examples I 
gave above.

> nm.c:static char value_format_64bit[] = "%016llx";

This is the only case where there actually appears to be a bug in the 
existing code.  OK to commit the patch below (untested) to fix it?

> prdbg.c:	sprintf (buf, "0x%llx", (unsigned long long) vma);

This has __MSVCRT__ conditionals.

> readelf.c:		  ? "%16.16llx  %16.16llx "
> readelf.c:		  : "%12.12llx  %12.12llx ",

This has __MSVCRT__ conditionals.

> strings.c:	        printf ("%7llx ", (unsigned long long) start);

This has __MSVCRT__ conditionals.

> print_vma will fail many tests due to leading 0s.

I was giving examples of how this should be implemented to work across 
hosts rather than saying that an existing function could be used without 
changes.

I'm aware that gold has several potentially problematic "ll" formats - 
it's quite possible that such diagnostics are not covered by the gold 
testsuite, and use of gold as a cross-linker on Windows hosts may have 
been limited.  (Also, whether a particular printf-family function ends up 
getting replaced by a MinGW version that supports "ll", or goes through to 
an MSVCRT version, may depend on the details of the MinGW version and 
which libraries you link with - MinGW supports various different versions 
of the Windows DLLs.)  But with my patch applied I don't see any in the 
binutils/ directory that are apparent from a grep.  In bfd/ there are some 
in coff-rs6000.c, and coffcode.h conditional on XCOFF64.  I don't see any 
problem uses in gas (without a proper conditional) or ld.

2010-10-02  Joseph Myers  <joseph@codesourcery.com>

	* nm.c (value_format_64bit): Define appropriately for __MSVCRT__.
	(set_print_radix): Update for __MSVCRT__ definition of
	value_format_64bit.

Index: nm.c
===================================================================
RCS file: /cvs/src/src/binutils/nm.c,v
retrieving revision 1.63
diff -u -p -r1.63 nm.c
--- nm.c	6 Jan 2010 08:48:19 -0000	1.63
+++ nm.c	2 Oct 2010 17:02:14 -0000
@@ -164,7 +164,11 @@ static char value_format_32bit[] = "%08l
 #if BFD_HOST_64BIT_LONG
 static char value_format_64bit[] = "%016lx";
 #elif BFD_HOST_64BIT_LONG_LONG
+#ifndef __MSVCRT__
 static char value_format_64bit[] = "%016llx";
+#else
+static char value_format_64bit[] = "%016I64x";
+#endif
 #endif
 static int print_width = 0;
 static int print_radix = 16;
@@ -284,7 +288,11 @@ set_print_radix (char *radix)
 #if BFD_HOST_64BIT_LONG
       value_format_64bit[5] = *radix;
 #elif BFD_HOST_64BIT_LONG_LONG
+#ifndef __MSVCRT__
       value_format_64bit[6] = *radix;
+#else
+      value_format_64bit[7] = *radix;
+#endif
 #endif
       other_format[3] = desc_format[3] = *radix;
       break;

-- 
Joseph S. Myers
joseph@codesourcery.com

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]