[patch] Avoid using strtoll

[patch] Avoid using strtoll

[Rafael Espindola]

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

The attached patch avoids using strtoll in gcc code. It also fixes the
assumption that long long is 64 bits.

If I understand it correctly, the configure system does its best to
give us a 64 bit type, so this patch should be portable.

OK if it boostraps and tests are OK?

gcc/lto
2009-11-18  Rafael Avila de Espindola  <espindola@google.com>

	PR42096
	* lto-elf.c (lto_elf_file_open): Use lto_parse_hex.
	* lto.c (lto_parse_hex): New.
	(lto_resolution_read): Use lto_parse_hex.
	* lto.h (lto_parse_hex): New.

2009-11-18  Rafael Avila de Espindola  <espindola@google.com>

	PR42096
	* lto-plugin.c (claim_file_handler): Print offsets in hex.

Cheers,
-- 
Rafael Ávila de Espíndola

[-- Attachment #2: strtoll.patch --]
[-- Type: text/x-diff, Size: 3342 bytes --]

diff --git a/gcc/lto/lto-elf.c b/gcc/lto/lto-elf.c
index 368d8d4..7c5453a 100644
--- a/gcc/lto/lto-elf.c
+++ b/gcc/lto/lto-elf.c
@@ -559,14 +559,8 @@ lto_elf_file_open (const char *filename, bool writable)
       fname = (char *) xmalloc (offset_p - filename + 1);
       memcpy (fname, filename, offset_p - filename);
       fname[offset_p - filename] = '\0';
-      offset_p++;
-      errno = 0;
-      offset = strtoll (offset_p, NULL, 10);
-      if (errno != 0)
-        {
-          error ("could not parse offset %s", offset_p);
-          goto fail;
-        }
+      offset_p += 3; /* skip the @0x */
+      offset = lto_parse_hex (offset_p);
       /* elf_rand expects the offset to point to the ar header, not the
          object itself. Subtract the size of the ar header (60 bytes).
          We don't uses sizeof (struct ar_hd) to avoid including ar.h */
diff --git a/gcc/lto/lto.c b/gcc/lto/lto.c
index 9cb7d65..4d7c307 100644
--- a/gcc/lto/lto.c
+++ b/gcc/lto/lto.c
@@ -249,6 +249,28 @@ lto_read_decls (struct lto_file_decl_data *decl_data, const void *data,
   lto_data_in_delete (data_in);
 }
 
+/* strtoll is not portable. */
+int64_t
+lto_parse_hex (const char *p) {
+  uint64_t ret = 0;
+  for (; *p != '\0'; ++p)
+    {
+      char c = *p;
+      unsigned char part;
+      ret <<= 4;
+      if (c >= '0' && c <= '9')
+        part = c - '0';
+      else if (c >= 'a' && c <= 'f')
+        part = c - 'a' + 10;
+      else if (c >= 'A' && c <= 'F')
+        part = c - 'A' + 10;
+      else
+        internal_error ("could not parse hex number");
+      ret |= part;
+    }
+  return ret;
+}
+
 /* Read resolution for file named FILE_NAME. The resolution is read from
    RESOLUTION. An array with the symbol resolution is returned. The array
    size is written to SIZE. */
@@ -280,15 +302,12 @@ lto_resolution_read (FILE *resolution, lto_file *file)
   if (file->offset != 0)
     {
       int t;
-      char offset_p[21];
-      long long offset;
-      t = fscanf (resolution, "@%20s", offset_p);
+      char offset_p[17];
+      int64_t offset;
+      t = fscanf (resolution, "@0x%16s", offset_p);
       if (t != 1)
         internal_error ("could not parse file offset");
-      errno = 0;
-      offset = strtoll(offset_p, NULL, 10);
-      if (errno != 0)
-        internal_error ("could not parse file offset");
+      offset = lto_parse_hex (offset_p);
       if (offset != file->offset)
         internal_error ("unexpected offset");
     }
diff --git a/gcc/lto/lto.h b/gcc/lto/lto.h
index 3b92b41..0c4305a 100644
--- a/gcc/lto/lto.h
+++ b/gcc/lto/lto.h
@@ -57,5 +57,6 @@ struct lto_section_slot
   size_t len;
 };
 
+int64_t lto_parse_hex (const char *p);
 
 #endif /* LTO_H */
diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
index 8cbafbc..4a8a0ff 100644
--- a/lto-plugin/lto-plugin.c
+++ b/lto-plugin/lto-plugin.c
@@ -551,7 +551,7 @@ claim_file_handler (const struct ld_plugin_input_file *file, int *claimed)
       Elf *archive;
       off_t offset;
       /* We pass the offset of the actual file, not the archive header. */
-      int t = asprintf (&objname, "%s@%" PRId64, file->name,
+      int t = asprintf (&objname, "%s@0x%" PRIx64, file->name,
                         (int64_t) file->offset);
       check (t >= 0, LDPL_FATAL, "asprintf failed");
       lto_file.name = objname;

Re: [patch] Avoid using strtoll

[Rainer Orth]

Rafael Espindola <espindola@google.com> writes:

> The attached patch avoids using strtoll in gcc code. It also fixes the
> assumption that long long is 64 bits.
>
> If I understand it correctly, the configure system does its best to
> give us a 64 bit type, so this patch should be portable.
>
> OK if it boostraps and tests are OK?
>
> gcc/lto
> 2009-11-18  Rafael Avila de Espindola  <espindola@google.com>
>
> 	PR42096

This is wrong: it should be PR bootstrap/42096 instead.

diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
index 8cbafbc..4a8a0ff 100644
--- a/lto-plugin/lto-plugin.c
+++ b/lto-plugin/lto-plugin.c
@@ -551,7 +551,7 @@ claim_file_handler (const struct ld_plugin_input_file *file, int *claimed)
       Elf *archive;
       off_t offset;
       /* We pass the offset of the actual file, not the archive header. */
-      int t = asprintf (&objname, "%s@%" PRId64, file->name,
+      int t = asprintf (&objname, "%s@0x%" PRIx64, file->name,

Unconditional use of PRI* is problematic, either: older platforms lack
it, although I'm not sure if this affects ELF platforms at the moment
(IRIX 5, probably).

cf. intl/loadmsgcat.c for a workaround, although that doesn't work as is
because you cannot use constructs like

# define PRId64 (sizeof (long) == 8 ? "ld" : "lld")

with string concatenation.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [patch] Avoid using strtoll

[Rafael Espindola]

> This is wrong: it should be PR bootstrap/42096 instead.

Will fix that. Thanks

> diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
> index 8cbafbc..4a8a0ff 100644
> --- a/lto-plugin/lto-plugin.c
> +++ b/lto-plugin/lto-plugin.c
> @@ -551,7 +551,7 @@ claim_file_handler (const struct ld_plugin_input_file *file, int *claimed)
>       Elf *archive;
>       off_t offset;
>       /* We pass the offset of the actual file, not the archive header. */
> -      int t = asprintf (&objname, "%s@%" PRId64, file->name,
> +      int t = asprintf (&objname, "%s@0x%" PRIx64, file->name,
>
> Unconditional use of PRI* is problematic, either: older platforms lack
> it, although I'm not sure if this affects ELF platforms at the moment
> (IRIX 5, probably).
>
> cf. intl/loadmsgcat.c for a workaround, although that doesn't work as is
> because you cannot use constructs like
>
> # define PRId64 (sizeof (long) == 8 ? "ld" : "lld")
>
> with string concatenation.

This code is part of the gold plugin, so I am not sure portability is
a problem. If it is, would this be OK if I used the define you
mentioned and 2 calls to asprintf?

>        Rainer
>
> --
> -----------------------------------------------------------------------------
> Rainer Orth, Center for Biotechnology, Bielefeld University
>

Cheers,
-- 
Rafael Ávila de Espíndola

Re: [patch] Avoid using strtoll

[Rainer Orth]

Rafael Espindola <espindola@google.com> writes:

>> diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
>> index 8cbafbc..4a8a0ff 100644
>> --- a/lto-plugin/lto-plugin.c
>> +++ b/lto-plugin/lto-plugin.c
>> @@ -551,7 +551,7 @@ claim_file_handler (const struct ld_plugin_input_file *file, int *claimed)
>>       Elf *archive;
>>       off_t offset;
>>       /* We pass the offset of the actual file, not the archive header. */
>> -      int t = asprintf (&objname, "%s@%" PRId64, file->name,
>> +      int t = asprintf (&objname, "%s@0x%" PRIx64, file->name,
>>
>> Unconditional use of PRI* is problematic, either: older platforms lack
>> it, although I'm not sure if this affects ELF platforms at the moment
>> (IRIX 5, probably).
>>
>> cf. intl/loadmsgcat.c for a workaround, although that doesn't work as is
>> because you cannot use constructs like
>>
>> # define PRId64 (sizeof (long) == 8 ? "ld" : "lld")
>>
>> with string concatenation.
>
> This code is part of the gold plugin, so I am not sure portability is
> a problem. If it is, would this be OK if I used the define you
> mentioned and 2 calls to asprintf?

It has been claimed that gold is supposed to be (or at least become)
portable and support even non-ELF platforms in the future.  Still, I
wouldn't clutter the code with two different invocations of asprintf(),
but instead provide a definition of PRId64/PRIx64 which tests for sizeof (long)
with preprocessor conditionals.  Leave it as is for the moment; I'm
checking how best to handle this for PR libffi/40701.

         Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University