[Bug translator/13486] New: pass-4 error (frame_base undeclared), inlined function argument

[Bug translator/13486] New: pass-4 error (frame_base undeclared), inlined function argument

[fche at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=13486

             Bug #: 13486
           Summary: pass-4 error (frame_base undeclared), inlined function
                    argument
           Product: systemtap
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: translator
        AssignedTo: systemtap@sourceware.org
        ReportedBy: fche@redhat.com
    Classification: Unclassified


Created attachment 6098
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6098
rmap.o from build tree

On a rawhide VM, the following script causes buggy loc2c code generation:


% stap -p4 -e 'probe kernel.function("try_to_unmap_cluster")
{println($mapcount)}' 
/var/tmp/stapEaoDzl/stap_18fc171ff2ff11b32cb9f4b595ad29e1_1254.c: In function
‘function__dwarf_tvar_get_mapcount_0’:
/var/tmp/stapEaoDzl/stap_18fc171ff2ff11b32cb9f4b595ad29e1_1254.c:175:16: error:
‘frame_base’ undeclared (first use in this function)
/var/tmp/stapEaoDzl/stap_18fc171ff2ff11b32cb9f4b595ad29e1_1254.c:175:16: note:
each undeclared identifier is reported only once for each function it appears
in
make[1]: *** [/var/tmp/stapEaoDzl/stap_18fc171ff2ff11b32cb9f4b595ad29e1_1254.o]
Error 1
make: *** [_module_/var/tmp/stapEaoDzl] Error 2

% dmesg|head

[    0.000000] Linux version 3.2.0-rc4-next-20111205 (fche@vm-rawhide-64) (gcc
version 4.6.2 20111027 (Red Hat 4.6.2-1) (GCC) ) #1 SMP Mon Dec 5 05:30:40 EST
2011

% readelf -w /notnfs/linux-2.6/mm/rmap.o

[...]
    <e6ac>   DW_AT_name        : (indirect string, offset: 0x1220):
try_to_unmap_cluster

    <e6b0>   DW_AT_decl_file   : 1      
    <e6b1>   DW_AT_decl_line   : 1315   
    <e6b3>   DW_AT_prototyped  : 1      
    <e6b3>   DW_AT_type        : <0x7c> 
    <e6b7>   DW_AT_inline      : 1      (inlined)
    <e6b8>   DW_AT_sibling     : <0xe796>       
 <2><e6bc>: Abbrev Number: 71 (DW_TAG_formal_parameter)
    <e6bd>   DW_AT_name        : (indirect string, offset: 0x30e9): cursor      
    <e6c1>   DW_AT_decl_file   : 1      
    <e6c2>   DW_AT_decl_line   : 1315   
    <e6c4>   DW_AT_type        : <0x11a>        
 <2><e6c8>: Abbrev Number: 71 (DW_TAG_formal_parameter)
    <e6c9>   DW_AT_name        : (indirect string, offset: 0x3fa5): mapcount    
    <e6cd>   DW_AT_decl_file   : 1      
    <e6ce>   DW_AT_decl_line   : 1315   
    <e6d0>   DW_AT_type        : <0xf1b>        
 <2><e6d4>: Abbrev Number: 64 (DW_TAG_formal_parameter)
    <e6d5>   DW_AT_name        : vma    
    <e6d9>   DW_AT_decl_file   : 1      
    <e6da>   DW_AT_decl_line   : 1316   
    <e6dc>   DW_AT_type        : <0x4d83>       

I don't see a location list for this "mapcount" variable, but that must just be 
my lyin' eyes.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug translator/13486] pass-4 error (frame_base undeclared), inlined function argument

[fche at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=13486

--- Comment #1 from Frank Ch. Eigler <fche at redhat dot com> 2011-12-08 20:14:16 UTC ---
Created attachment 6099
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6099
generated C code, error on line #175

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug translator/13486] pass-4 error (frame_base undeclared), inlined function argument

[jistone at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=13486

Josh Stone <jistone at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jistone at redhat dot com

--- Comment #2 from Josh Stone <jistone at redhat dot com> 2012-09-27 00:57:17 UTC ---
(In reply to comment #0)
> I don't see a location list for this "mapcount" variable, but that must just be 
> my lyin' eyes.

I believe that's because you only quoted the "subprogram" section for the
inline, which is basically just info shared across all instances.  You need the
particular "inlined_subroutine" to get actual location lists.

Your rmap.o shows this at ead9, with mapcount at eb03 and location list 5dcb. 
This is an implicit_pointer to die e858 with location list 58df.  And part of
that list indeed references fb_reg -172, as in your generated C.

I can't reproduce the issue with your script on a current F17 kernel, as I get
just "not accessible at this address".  But I just ran into this elsewhere:

$ stap -e 'probe process.function("seen_triple") { println($ht) }' -c df -p4
/tmp/stapPYTjE4/stap_7fb9f85209eb688e311d35324c1e6749_1395_src.c: In function
‘function__dwarf_tvar_get_ht_0’:
/tmp/stapPYTjE4/stap_7fb9f85209eb688e311d35324c1e6749_1395_src.c:118:16: error:
‘frame_base’ undeclared (first use in this function)

And the symptoms are the same, an inline function parameter resolving to an
implicit_pointer to something else that references fb_reg.

I figured out that loc2c.c:c_emit_location() only emits code for frame_base if
it's on the first loc of the chain.  But in this case, the loc_implicit_pointer
has no frame_base, yet its ->pointer.target does.  That target comes later in
the chain, thus we get a reference to frame_base without defining it.

I'm testing this patch to look if *any* of the loc chain needs frame_base, and
so far it seems to work:

diff --git a/loc2c.c b/loc2c.c
index 16b8c8e..7c4ec60 100644
--- a/loc2c.c
+++ b/loc2c.c
@@ -2538,9 +2538,13 @@ c_emit_location (FILE *out, struct location *loc, int
indent,
   bool deref = false;
   *max_stack = 0;

-  if (loc->frame_base != NULL)
-    emit_loc_value (out, loc->frame_base, indent, "frame_base", true,
-                   &deref, max_stack);
+  for (l = loc; l != NULL; l = l->next)
+    if (l->frame_base != NULL)
+      {
+       emit_loc_value (out, l->frame_base, indent, "frame_base", true,
+                       &deref, max_stack);
+       break;
+      }

   for (; loc->next != NULL; loc = loc->next)
     switch (loc->type)

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug translator/13486] pass-4 error (frame_base undeclared), inlined function argument

[mjw at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=13486

Mark Wielaard <mjw at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mjw at redhat dot com

--- Comment #3 from Mark Wielaard <mjw at redhat dot com> 2012-09-27 08:06:34 UTC ---
Just because I was curious what was going on. In the case you found (the ht
variable inside an inlined function seen_triple in df), the DW_AT_location is
expressed as an GNU_implicit_pointer. The GNU_implicit_pointer is expressed as
the location of another DIE (plus some offset). And in this case that DIE has
as location fbreg -240. So, that then does need the frame_base.

I think your patch handles this correctly. But you could consider moving the
check up into the for (l = loc; l != NULL; l = l->next) loop just above it that
checks the types. You can probably just check whether to output the frame_base
at the same time instead of looping through the location chain twice. Although
the chains aren't very deep, so it probably doesn't matter.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug translator/13486] pass-4 error (frame_base undeclared), inlined function argument

[jistone at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=13486

Josh Stone <jistone at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #4 from Josh Stone <jistone at redhat dot com> 2012-09-27 16:52:41 UTC ---
(In reply to comment #3)
> I think your patch handles this correctly. But you could consider moving the
> check up into the for (l = loc; l != NULL; l = l->next) loop just above it that
> checks the types. You can probably just check whether to output the frame_base
> at the same time instead of looping through the location chain twice. Although
> the chains aren't very deep, so it probably doesn't matter.

Thanks for checking.  I did consider moving it into the loop you mention, but
decided not to for two reasons.  First, I didn't want to perturb the relative
order of the addr and frame_base declarations.  Second, we only want a single
frame_base declaration, even if multiple parts of the chain use it, so if we
can't break the loop, then another tracking variable is needed.  It seems
simpler to add a new loop iteration just for the frame_base.

Pushed as commit b0b86028.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.