This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap 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: [RFC updated PATCH 1/2] Bug Translator 3016 : Error accessing members of anonymous structs / unions


Hi Masami,

Masami Hiramatsu wrote:
Hi Prerna,

Prerna Saxena wrote:
Hi all,
I'm posting patch 1 of 2 after fixing a bug which was causing it to fail on x86_64.


@Masami, thanks for pointing me to it :-)

Masami Hiramatsu wrote:
I think below code is a suspicious code.

+ Dwarf_Die *result_die = translate_components(pool, tail,
pc, components, &temp_die, &temp_die, &temp_attr );
Since temp_die is just a local variable, I think secound &temp_die(6th argument)
should be die_mem as same as original function.

As far as I can see, your patch still use a local variable for returning memory buffer(temp_die_2).

+ Dwarf_Die temp_die_2;
[snip]
+ Dwarf_Die *result_die = translate_components(pool, tail, pc, components, &temp_die, &temp_die_2, &temp_attr );
+

If translate_components found a matched die, it stores that die information into die_mem(temp_die_2, in your code) and returns the address of die_mem(&temp_die_2) to caller.

+ if (result_die != NULL)
+ { + *attr_mem = temp_attr;
+ return result_die;
+ }

In your code, if result_die (= &temp_die_2) is not NULL, returns it to caller again. This means, translate_components returns it's local memory on stack which will be used by other functions in the future. (Note: Without your patch, translate_components returns vardie or die_mem, both of which are passed from caller and not on local memory.)

So, I think you must use die_mem which is passed by other functions(
literal_stmt_for_local or literal_stmt_for_return) instead of &temp_die_2.

Thank you,

Agree, but making "die_mem" as the 6th arg will disturb the call flow.
This is because in each recursive call, the 6th arg is overwritten by a die which is pointed to by DW_AT_type attribute of the original attr_mem. If die_mem is re-used as 6th arg instead of a new variable, the old contents of die (in the parent recursive call) will also be lost as both die & die_mem point to the same location, (whose contents would be overwritten). This is not a problem if search in a branch has succeeded-- but in case a search path fails and a new branch needs to be tried, this will deem it impossible.


I agree with your concern about "temp_die_2" being local memory on the stack which may be reused, so I've fixed it by copying the contents of "temp_die_2" to "die_mem" in case of a successful match. This should take care of memory errors.

Regards,

--

Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India


Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>

Index: systemtap/tapsets.cxx
===================================================================
--- systemtap.orig/tapsets.cxx
+++ systemtap/tapsets.cxx
@@ -1866,7 +1866,15 @@ struct dwflpp
   {
     Dwarf_Die *die = vardie;
     Dwarf_Die struct_die;
+    Dwarf_Attribute temp_attr;
+
     unsigned i = 0;
+
+    static unsigned int func_call_level ;
+    static unsigned int dwarf_error_flag ; // indicates current error is dwarf error
+    static unsigned int dwarf_error_count ; // keeps track of no of dwarf errors
+    static semantic_error saved_dwarf_error("");
+
     while (i < components.size())
       {
         /* XXX: This would be desirable, but we don't get the target_symbol token,
@@ -1924,9 +1932,7 @@ struct dwflpp
 	    switch (dwarf_child (die, die_mem))
 	      {
 	      case 1:		/* No children.  */
-		throw semantic_error ("empty struct "
-				      + string (dwarf_diename_integrate (die) ?: "<anonymous>"));
-		break;
+		return NULL;
 	      case -1:		/* Error.  */
 	      default:		/* Shouldn't happen */
 		throw semantic_error (string (typetag == DW_TAG_union_type ? "union" : "struct")
@@ -1941,14 +1947,60 @@ struct dwflpp
 	    while (dwarf_tag (die) != DW_TAG_member
 		   || ({ const char *member = dwarf_diename_integrate (die);
 		       member == NULL || string(member) != components[i].second; }))
+	    {
+	      if ( dwarf_diename (die) == NULL )  // handling Anonymous structs/unions
+		{ 
+		   Dwarf_Die temp_die = *die;
+		   Dwarf_Die temp_die_2;
+
+		   try 
+		    {		   
+		      if (!dwarf_attr_integrate (&temp_die, DW_AT_type, &temp_attr))
+                       { 
+			  dwarf_error_flag ++ ;
+			  dwarf_error_count ++;
+			  throw semantic_error(" Error in obtaining type attribute for "+ string(dwarf_diename(&temp_die)?:"<anonymous>"));
+                	}
+
+	              if ( !dwarf_formref_die (&temp_attr, &temp_die))
+        	        { 
+			  dwarf_error_flag ++ ;
+			  dwarf_error_count ++;
+			  throw semantic_error(" Error in decoding DW_AT_type attribute for " + string(dwarf_diename(&temp_die)?:"<anonymous>")); 
+                	}
+
+		      func_call_level ++ ;
+
+                      Dwarf_Die *result_die = translate_components(pool, tail, pc, components, &temp_die, &temp_die_2, &temp_attr );
+
+		      func_call_level -- ;
+
+		      if (result_die != NULL)
+			{ 
+			  memcpy(die_mem, &temp_die_2, sizeof(Dwarf_Die));
+			  *attr_mem = temp_attr;
+			  return result_die;
+			}
+		     }
+		   catch (const semantic_error& e)
+		     { 
+			if ( !dwarf_error_flag ) //not a dwarf error
+				throw;
+			else
+				{
+				  dwarf_error_flag = 0 ;
+				  saved_dwarf_error = e ;
+				}
+		     }
+		}
 	      if (dwarf_siblingof (die, die_mem) != 0)
-	      {
-		  stringstream alternatives;
-		  print_members (&struct_die, alternatives);
-		  throw semantic_error ("field '" + components[i].second
-					+ "' not found (alternatives:"
-					+ alternatives.str () + ")");
-	      }
+		{ 
+		  if ( func_call_level == 0 && dwarf_error_count ) // this is parent call & a dwarf error has been reported in a branch somewhere
+			throw semantic_error( saved_dwarf_error );
+		  else	
+		  	 return NULL;
+		}
+	    }
 
 	    if (dwarf_attr_integrate (die, DW_AT_data_member_location,
 				      attr_mem) == NULL)
@@ -2230,6 +2282,8 @@ struct dwflpp
     Dwarf_Die die_mem, *die = NULL;
     die = translate_components (&pool, &tail, pc, components,
 				&vardie, &die_mem, &attr_mem);
+    if(!die)
+	throw semantic_error("Translation failure");
 
     /* Translate the assignment part, either
        x = $foo->bar->baz[NN]
@@ -2297,6 +2351,8 @@ struct dwflpp
     Dwarf_Die die_mem, *die = NULL;
     die = translate_components (&pool, &tail, pc, components,
 				vardie, &die_mem, &attr_mem);
+    if(!die)
+	throw semantic_error("Translation Failure");
 
     /* Translate the assignment part, either
        x = $return->bar->baz[NN]

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