This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc 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] abilist.awk: Treat .tdata like .tbss and reject unknown combinations.


On 11/30/18 10:48 AM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>> On 11/27/18 11:42 AM, Joseph Myers wrote:
>>> I'm now seeing compilation tests failing for i686-gnu.
>>>
>>> LC_ALL=C gawk -f ../scripts/abilist.awk /scratch/jmyers/glibc-bot/build/glibcs/i686-gnu/glibc/mach/libmachuser.dynsym > /scratch/jmyers/glibc-bot/build/glibcs/i686-gnu/glibc/mach/libmachuser.symlistT
>>> ../Makerules:1344: recipe for target '/scratch/jmyers/glibc-bot/build/glibcs/i686-gnu/glibc/mach/libmachuser.symlist' failed
>>> make[3]: *** [/scratch/jmyers/glibc-bot/build/glibcs/i686-gnu/glibc/mach/libmachuser.symlist] Error 1
>>>
>>> These tests for libmachuser and libhurduser are XFAILed in 
>>> sysdeps/mach/hurd/i386/Makefile, but that's for the comparison failing, 
>>> whereas now the generation of the .symlist file fails ("ERROR: Unable to 
>>> handle this type of symbol." - it would be helpful for that message to say 
>>> exactly what the line is that generates the error) and so "make check" 
>>> exits early.
>>  
>> I did not run a build-many-glibc's test for this change, thinking that
>> it wouldn't impact any other machine/OS combinations.
>>
>> I have just started a build-many-glibc's run to see if anything else
>> fails.
>>
>> I'll add some debugging code and see if I can see what's wrong.
> 
> It's related to this from hurd/Versions, which looks rather questionable
> to me because I assumed that _end ought to be a hidden symbol:
> 
>     # necessary for the Hurd brk implementation
>     _end;
 
The default linker script does not mark them hidden, but from within
glibc's dynamic loader we add attribute_hidden to the reference, and
we don't generally want that symbol to be visible to the application
(even if it is in the implementation's name space).

I think this is a defect in:
sysdeps/mach/hurd/brk.c
where we have extern but without attribute_hidden.

In 2002 with 5c82e15e8646cd7d229bcd8287d01875e12df0b3 we started
marking _end as hidden.

> I checked the attached patch on i686-gnu (cross-test), x86_64-linux-gnu,
> i686-linux-gnu, powerpc64le-linux-gnu, ia64-linux-gnu (cross-test).
> 
> Thanks,
> Florian
> 
> -----
> scripts/abilist.awk: Handle special _end symbol for Hurd
> 
> Hurd has this in libc.so:
> 
> 0024db9c g    D  .bss   00000000  GLIBC_2.2.6 _end
> 
> This g/D combination was not recognized before.
> 
> 2018-11-30  Florian Weimer  <fweimer@redhat.com>
> 
> 	* scripts/abilist.awk: Print "0x0" for size 0. Handle "g"/"D".
> 	Extend error logging.
> 	* sysdeps/mach/hurd/i386/libc.abilist (GLIBC_2.2.6): Adjust _end
> 	symbol.

LGTM.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> 
> diff --git a/scripts/abilist.awk b/scripts/abilist.awk
> index b40be91f82..a43400d5b4 100644
> --- a/scripts/abilist.awk
> +++ b/scripts/abilist.awk
> @@ -42,7 +42,11 @@ $2 == "g" || $2 == "w" && (NF == 7 || NF == 8) {
>    type = $3;
>    size = $5;
>    sub(/^0*/, "", size);
> -  size = " 0x" size;
> +  if (size == "") {
> +      size = " 0x0";
> +  } else {
> +      size = " 0x" size;
> +  }

OK. Handle zero sized symbols like _end (entirely possible in other
instances too).

>    version = $6;
>    symbol = $NF;
>    gsub(/[()]/, "", version);
> @@ -73,6 +77,9 @@ $2 == "g" || $2 == "w" && (NF == 7 || NF == 8) {
>    else if ($4 == "*ABS*") {
>      next;
>    }
> +  else if (type == "D") {
> +    # Accept unchanged.
> +  }

OK. Handle initialized data section symbols being part of the ABI.

>    else if (type == "DO") {
>      type = "D";
>    }
> @@ -89,7 +96,7 @@ $2 == "g" || $2 == "w" && (NF == 7 || NF == 8) {
>      size = "";
>    }
>    else {
> -    print "ERROR: Unable to handle this type of symbol."
> +    print "ERROR: Unable to handle this type of symbol:", $0

OK, add a bit more debugging.

>      exit 1
>    }
>  
> diff --git a/sysdeps/mach/hurd/i386/libc.abilist b/sysdeps/mach/hurd/i386/libc.abilist
> index d4c4a91c84..f3993cf994 100644
> --- a/sysdeps/mach/hurd/i386/libc.abilist
> +++ b/sysdeps/mach/hurd/i386/libc.abilist
> @@ -554,7 +554,7 @@ GLIBC_2.2.6 __xstat64 F
>  GLIBC_2.2.6 _authenticate F
>  GLIBC_2.2.6 _dl_mcount_wrapper F
>  GLIBC_2.2.6 _dl_mcount_wrapper_check F
> -GLIBC_2.2.6 _end GLIBC_2.2.6 g ? D .bss 00000000
> +GLIBC_2.2.6 _end D 0x0

OK. Ah! If I'd been clever I would have looked for existing instances of the now
removed fallback in ABI files.

[carlos@athas glibc]$ find . -name '*.abilist' | xargs grep '?'
./sysdeps/mach/hurd/i386/libc.abilist:GLIBC_2.2.6 _end GLIBC_2.2.6 g ? D .bss 00000000

There is only one, and you fixed it.

>  GLIBC_2.2.6 _environ D 0x4
>  GLIBC_2.2.6 _exit F
>  GLIBC_2.2.6 _flushlbf F
> 


-- 
Cheers,
Carlos.


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