Ticket #1981 (closed todo: fixed)

Opened 4 years ago

Last modified 4 years ago

Many new warnings like: warning: variable ‘interp’ might be clobbered by ‘longjmp’ or ‘vfork’

Reported by: dukeleto Owned by: dukeleto
Priority: trivial Milestone:
Component: build Version: 3.0.0
Severity: low Keywords:
Cc: Language:
Patch status: Platform: all

Description (last modified by coke) (diff)

This is happening on a 64 bit linux with gcc 4.4.3

src/embed/pmc.c: In function ‘Parrot_api_pmc_find_method’:
src/embed/pmc.c:584: warning: variable ‘interp’ might be clobbered by ‘longjmp’ or ‘vfork’

Change History

  Changed 4 years ago by coke

These date from whiteknight's API changes, IIRC.

follow-up: ↓ 3   Changed 4 years ago by nwellnhof

We have to declare variables used before setjmp as volatile.

in reply to: ↑ 2   Changed 4 years ago by plobsing

Replying to nwellnhof:

We have to declare variables used before setjmp as volatile.

That's what we should if we want to appease a stupid compiler that is trying to act smart. This is not an approach I think we should take.

The variable in question, interp, is not used after a longjmp() back into these function, and so will not cause problems even if it is clobbered.

The relevant disassembly (from x86_64-linux, gcc 4.5.2 -O3) for such functions is:

   0x00000000001800bf <+95>:	callq  0x125380 <_setjmp@plt>
   0x00000000001800c4 <+100>:	test   %eax,%eax
   0x00000000001800c6 <+102>:	je     0x180128 <Parrot_api_toggle_gc+200>    # jump away on local return
   0x00000000001800c8 <+104>:	mov    0x4b9219(%rip),%rax        # 0x6392e8
   0x00000000001800cf <+111>:	mov    0x8(%rsp),%rdx
   0x00000000001800d4 <+116>:	cmp    (%rax),%rdx
   0x00000000001800d7 <+119>:	je     0x180188 <Parrot_api_toggle_gc+296>
   0x00000000001800dd <+125>:	cmpb   $0x0,0x1f(%rsp)
   0x00000000001800e2 <+130>:	je     0x180188 <Parrot_api_toggle_gc+296>
   0x00000000001800e8 <+136>:	mov    0x10(%rdx),%rax
   0x00000000001800ec <+140>:	mov    (%rax),%rax
   0x00000000001800ef <+143>:	movq   $0x0,0x180(%rax)
   0x00000000001800fa <+154>:	cmpq   $0x0,0x190(%rax)
   0x0000000000180102 <+162>:	sete   %al
   0x0000000000180105 <+165>:	add    $0x108,%rsp
   0x000000000018010c <+172>:	movzbl %al,%eax
   0x000000000018010f <+175>:	retq

   ...

   0x0000000000180188 <+296>:	xor    %eax,%eax
   0x000000000018018a <+298>:	jmpq   0x1800ef <Parrot_api_toggle_gc+143>

This shows that all values used after a non-local setjmp return originate from %eax, %rip, and %rsp, all of which will be initialized by setjmp.

This is not a bug in Parrot. If these messages bother people, they should get GCC to stop detecting errors that aren't actually there.

  Changed 4 years ago by dukeleto

  • status changed from new to assigned
  • severity changed from medium to low
  • priority changed from normal to trivial
  • platform set to all
  • owner changed from whiteknight to dukeleto
  • type changed from bug to todo

I have an action plan:

1) See if this bug in GCC still occurs in their latest release/trunk (which I have access to, already compiled, on the GCC Compile Farm) 2) If it does, I will send a bug report to them and close this ticket with a link to their bug report. 3) If it has been fixed by them already, I will close this ticket. 4) If they don't consider this a bug, I will cry a single tear and close this ticket.

Thanks for your detailed analysis, plobsing++

  Changed 4 years ago by nwellnhof

There already is a GCC bug for that issue (or something very similar):

 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21161

It has been reported over 5 years ago, so this probably won't get fixed soon.

  Changed 4 years ago by cotto

Having shed a single tear on dukeleto's behalf, I'd say +1 to using volatile to get gcc to shut up. If using volatile is likely to degrade performance, we should at least add a comment to the relevant code explaining why gcc is wrong.

  Changed 4 years ago by coke

  • description modified (diff)

We could also set config/auto/warnings.pm to not warn about this. (either everywhere or just on these files.)

  Changed 4 years ago by NotFound

  • status changed from assigned to closed
  • resolution set to fixed

Fixed without adding volatile in 02ca078 and ced54a5

 http://lists.apple.com/archives/xcode-users/2003/Dec//msg00050.html suggest:

"it's normally enough just to change their scope"

and in this case the variables doesn't need a wider scope.

Note: See TracTickets for help on using tickets.