This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


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

Re: Patch for GDB top of trunk



Nat, in theory, and for future reference, this patch should be split
into 3 separate ones, because it addresses 3 separate issues.

I agree with the mi-main.c patch, and the xfree patch, they can go in,
but I don't understand the other mi-cmd-disassemble patch.  Can you
provide some more details on the circumstances in which this occurs?

Also, do you have an assignment on file with the FSF?

Thanks
Elena


Nat Gurumoorthy writes:
 > Description of the bugs:
 > 	In mi-main.c there are places where
 > "mi_cmd_data_list_register_values" returns MI_CMD_ERROR.
 > At each of those places ui_out_tuple_end needs to be called
 > to balance calls to ui_out_tuple_begin.
 > 
 > 	In mi-cmd-disas.c in the function "mi_cmd_disassemble"
 > there is for loop which converts le entries to mle entries. The
 > mle[i].end_pc is created by
 > mld[i].end_pc = le[i+1].pc. There will be cases where 
 > end_pc >= high and hence has to wrong address for the last line
 > of a function. We need to clamp by calling find_pc_line and letting
 > it figure out the end_pc for this line and copying it in place.
 > 
 > 	Further down in the same funtion there is a for loop to print
 > each assembly line for mixed mode printing. You enter the for
 > loop file filename = NULL and name = NULL. 
 > build_address_symbolic is called and it may or may not allocate
 > memory for filename and name. xfree routine is called to free this
 > allocated memory if filename or name != NULL. These pointers
 > need to be set to NULL before you go through the loop one more time.
 > If build_address_symbolic fails to allocate memory xfree will get
 > called with stale pointers.
 > 
 > Regards
 > Nat.
 > 
 > PS. I do not have test cases. I ran into these while chasing GDB
 > crashes in Rehat's UBICOM ip2k port of GDB. I then looked up
 > to see if the main GDB trunk had these problems and hence
 > this patch.
 > 
 > 
 > Changelog entry
 > 2001-06-11 Nat Gurumoorthy <nat.gurumoorthy@ubicom.com>
 > 
 > 	* mi-main.c (mi_cmd_data_list_changed_registers,
 > 	mi_cmd_data_list_register_values: Ensure
 > 	ui_out_tuple_end gets called enough number of times before 
 > 	return MI_CMD_ERROR happens.
 > 	
 > 	* mi-cmd-disas.c (mi_cmd_disassemble): Make sure "filename"
 > 	and "name" pointers get set to NULL after call to xfree. This
 > 	is to prevent calls to xfree with stale pointers if call to
 > 	build_address_symbolic fails to allocate memory for either
 > 	of these.
 > 	(mi_cmd_disassemble): Clamp end_pc for a mle entry if 
 > 	end_pc is >= high. GDB will face this issue if a file has
 > 	code from different sections and the addresses of the sections
 > 	scrambles the line number ordering.
 > 
 > 
 > Patch
 > 
 > 
 > Index: gdb/mi/mi-cmd-disas.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/mi/mi-cmd-disas.c,v
 > retrieving revision 1.10
 > diff -c -3 -p -r1.10 mi-cmd-disas.c
 > *** mi-cmd-disas.c	2001/05/12 04:08:24	1.10
 > --- mi-cmd-disas.c	2001/06/12 02:49:26
 > *************** mi_cmd_disassemble (char *command, char 
 > *** 337,342 ****
 > --- 337,349 ----
 >   	    out_of_order = 1;
 >   	  mle[newlines].start_pc = le[i].pc;
 >   	  mle[newlines].end_pc = le[i + 1].pc;
 > + 
 > + 	  /* check if end_pc is >= high and clamp it */
 > + 	  if(mle[newlines].end_pc >= high)
 > + 	    {
 > + 	      sal = find_pc_line (le[i].pc, 0);
 > + 	      mle[newlines].end_pc = sal.end;
 > + 	    }
 >   	  newlines++;
 >   	}
 >   
 > *************** mi_cmd_disassemble (char *command, char 
 > *** 429,437 ****
 >   		  ui_out_field_int (uiout, "offset", offset);
 >   		}
 >   	      if (filename != NULL)
 > ! 		xfree (filename);
 >   	      if (name != NULL)
 > ! 		xfree (name);
 >   
 >   	      ui_file_rewind (stb->stream);
 >   	      pc += (*tm_print_insn) (pc, &di);
 > --- 436,450 ----
 >   		  ui_out_field_int (uiout, "offset", offset);
 >   		}
 >   	      if (filename != NULL)
 > ! 		{
 > ! 		  xfree (filename);
 > ! 		  filename = NULL;
 > ! 		}
 >   	      if (name != NULL)
 > ! 		{
 > ! 		  xfree (name);
 > ! 		  name = NULL;
 > ! 		}
 >   
 >   	      ui_file_rewind (stb->stream);
 >   	      pc += (*tm_print_insn) (pc, &di);
 > Index: gdb/mi/mi-main.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/mi/mi-main.c,v
 > retrieving revision 1.14
 > diff -c -3 -p -r1.14 mi-main.c
 > *** mi-main.c	2001/05/12 04:08:24	1.14
 > --- mi-main.c	2001/06/12 02:49:27
 > *************** mi_cmd_data_list_changed_registers (char
 > *** 339,344 ****
 > --- 339,345 ----
 >   	    {
 >   	      xasprintf (&mi_error_message,
 >   			 "mi_cmd_data_list_changed_registers: Unable to read
 > register contents.");
 > + 	      ui_out_tuple_end (uiout);
 >   	      return MI_CMD_ERROR;
 >   	    }
 >   	  else if (changed)
 > *************** mi_cmd_data_list_changed_registers (char
 > *** 361,366 ****
 > --- 362,368 ----
 >   	    {
 >   	      xasprintf (&mi_error_message,
 >   			 "mi_cmd_data_list_register_change: Unable to read
 > register contents.");
 > + 	      ui_out_tuple_end (uiout);
 >   	      return MI_CMD_ERROR;
 >   	    }
 >   	  else if (changed)
 > *************** mi_cmd_data_list_changed_registers (char
 > *** 369,374 ****
 > --- 371,377 ----
 >         else
 >   	{
 >   	  xasprintf (&mi_error_message, "bad register number");
 > + 	  ui_out_tuple_end (uiout);
 >   	  return MI_CMD_ERROR;
 >   	}
 >       }
 > *************** mi_cmd_data_list_register_values (char *
 > *** 447,455 ****
 >   	  ui_out_tuple_begin (uiout, NULL);
 >   	  ui_out_field_int (uiout, "number", regnum);
 >   	  result = get_register (regnum, format);
 > - 	  if (result == -1)
 > - 	    return MI_CMD_ERROR;
 >   	  ui_out_tuple_end (uiout);
 >   	}
 >       }
 >   
 > --- 450,461 ----
 >   	  ui_out_tuple_begin (uiout, NULL);
 >   	  ui_out_field_int (uiout, "number", regnum);
 >   	  result = get_register (regnum, format);
 >   	  ui_out_tuple_end (uiout);
 > + 	  if (result == -1)
 > + 	    {
 > + 	      ui_out_tuple_end (uiout);
 > + 	      return MI_CMD_ERROR;
 > + 	    }
 >   	}
 >       }
 >   
 > *************** mi_cmd_data_list_register_values (char *
 > *** 466,478 ****
 >   	  ui_out_tuple_begin (uiout, NULL);
 >   	  ui_out_field_int (uiout, "number", regnum);
 >   	  result = get_register (regnum, format);
 > - 	  if (result == -1)
 > - 	    return MI_CMD_ERROR;
 >   	  ui_out_tuple_end (uiout);
 >   	}
 >         else
 >   	{
 >   	  xasprintf (&mi_error_message, "bad register number");
 >   	  return MI_CMD_ERROR;
 >   	}
 >       }
 > --- 472,488 ----
 >   	  ui_out_tuple_begin (uiout, NULL);
 >   	  ui_out_field_int (uiout, "number", regnum);
 >   	  result = get_register (regnum, format);
 >   	  ui_out_tuple_end (uiout);
 > + 	  if (result == -1)
 > + 	    {
 > + 	      ui_out_tuple_end (uiout);
 > + 	      return MI_CMD_ERROR;
 > + 	    }
 >   	}
 >         else
 >   	{
 >   	  xasprintf (&mi_error_message, "bad register number");
 > + 	  ui_out_tuple_end (uiout);
 >   	  return MI_CMD_ERROR;
 >   	}
 >       }
 > 


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