Re: [PATCH] Fix handling of null stap semaphores

[Simon Marchi <simark@simark.ca>]

On 2019-12-30 10:38 a.m., George Barrett wrote:
> On Sun, Dec 29, 2019 at 01:58:46PM -0500, Simon Marchi wrote:
>> Though the ChangeLog should mention the modified functions, I propose to use
>> this:
>>
>> 	* stap-probe.c (stap_modify_semaphore): Don't check for null
>> 	semaphores.
>> 	(stap_probe::set_semaphore, stap_probe::clear_semaphore): Check
>> 	for null semaphores.
> 
> Ack.
> 
>> I'd really like if we could have a test for this, so that eventual refactors
>> don't re-introduce this bug.  Perhaps the gdb.base/stap-probe.exp test could
>> be enhanced to test that the ELF magic number hasn't been changed?
> 
> I was secretly hoping no-one would notice the absence of a test case ;)

Yeah, I know writing a test case is not the most fun part, but it pays in the
long run.  I can help if you are not familiar enough with tcl/expect/dejagnu.

>> One difficulty is finding out where it is, I don't know if there's a GDB
>> command that will compute that directly.  One way is to take the address of
>> a global variable before and after starting the process, and see how it has
>> been relocated, that would be the base of the image:
>>
>> (gdb) p &some_global
>> $1 = (int *) 0x402c <some_global>
>> (gdb) start
>> Temporary breakpoint 1 at 0x111d: file test.c, line 9.
>> Starting program: /home/simark/src/aoc/08/p2/a.out
>>
>> Temporary breakpoint 1, main () at test.c:9
>> 9	  for (i = 0; i < 1000; i++) {
>> (gdb) p &some_global
>> $2 = (int *) 0x55555555802c <some_global>
>> (gdb) p/x 0x55555555802c - 0x402c
>> $3 = 0x555555554000
>> (gdb) p (*(char*) 0x555555554000)@4
>> $4 = "\177ELF"
> 
> Is it too glibc-specific to use _r_debug? Something like the following:
> 
> (gdb) start
> Temporary breakpoint 1 at 0x4e60: file ../src/ls.c, line 1451.
> Starting program: /usr/bin/ls
> 
> Temporary breakpoint 1, main (argc=1, argv=0x7fffffffd8d8) at ../src/ls.c:1451
> (gdb) p/x _r_debug.r_map->l_addr
> $1 = 0x555555554000
> (gdb) p (*(char*) _r_debug.r_map->l_addr)@4
> $2 = "\177ELF"

Hmm, it is not only glibc-specific (I believe?), but it also requires having glibc
debug symbols installed.  I just tried in an Alpine docker image (which uses musl),
and the address subtraction method works.  So between the two methods, I'd prefer
the address subtraction, since it works on more libc's, and it works without debug
info for the libc.

Simon