[PATCH] Update comments in scan_partial_symbols and add_partial_subprogram

[PATCH] Update comments in scan_partial_symbols and add_partial_subprogram

[Yao Qi]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="yes", Size: 6874 bytes --]

I read comment of scan_partial_symbols about NEED_PC and how *LOWPC
and *HIGHPC are updated:

   DW_AT_ranges).  If NEED_PC is set, then this function will set
   *LOWPC and *HIGHPC to the lowest and highest PC values found in CU
   and record the covered ranges in the addrmap.

NEED_PC is only used in the callee of scan_partial_symbols,
add_partial_subprogram,

  if (pdi->tag == DW_TAG_subprogram)
    {
      if (pdi->has_pc_info)
        {
          if (pdi->lowpc < *lowpc)
            *lowpc = pdi->lowpc;
          if (pdi->highpc > *highpc)
            *highpc = pdi->highpc;
	  if (need_pc)

*LOWPC and *HIGHPC is updated regardless of NEED_PC.  When NEED_PC is
true, addrmap is updated.  It would be clear to rename NEED_PC to
SET_ADDRMAP.  That is what this patch does.  Beside this, this patch
also adjust comments in related functions.

gdb:

2014-08-22  Yao Qi  <yao@codesourcery.com>

	* dwarf2read.c (scan_partial_symbols):  Update comments.
	Rename argument 'need_pc' with 'set_addrmap'.
	（add_partial_namespace): Rename argument 'need_pc' with
	'set_addrmap'.
	(add_partial_module): Likewise.
	(add_partial_subprogram): Likewise.  Update comments.
---
 gdb/dwarf2read.c | 40 +++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 9086287..b4d53c8 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1387,10 +1387,10 @@ static void add_partial_symbol (struct partial_die_info *,
 
 static void add_partial_namespace (struct partial_die_info *pdi,
 				   CORE_ADDR *lowpc, CORE_ADDR *highpc,
-				   int need_pc, struct dwarf2_cu *cu);
+				   int set_addrmap, struct dwarf2_cu *cu);
 
 static void add_partial_module (struct partial_die_info *pdi, CORE_ADDR *lowpc,
-				CORE_ADDR *highpc, int need_pc,
+				CORE_ADDR *highpc, int set_addrmap,
 				struct dwarf2_cu *cu);
 
 static void add_partial_enumeration (struct partial_die_info *enum_pdi,
@@ -6542,15 +6542,15 @@ create_all_comp_units (struct objfile *objfile)
 }
 
 /* Process all loaded DIEs for compilation unit CU, starting at
-   FIRST_DIE.  The caller should pass NEED_PC == 1 if the compilation
+   FIRST_DIE.  The caller should pass SET_ADDRMAP == 1 if the compilation
    unit DIE did not have PC info (DW_AT_low_pc and DW_AT_high_pc, or
-   DW_AT_ranges).  If NEED_PC is set, then this function will set
-   *LOWPC and *HIGHPC to the lowest and highest PC values found in CU
-   and record the covered ranges in the addrmap.  */
+   DW_AT_ranges).  See the comments of add_partial_subprogram on how
+   SET_ADDRMAP is used and how *LOWPC and *HIGHPC are updated.  */
 
 static void
 scan_partial_symbols (struct partial_die_info *first_die, CORE_ADDR *lowpc,
-		      CORE_ADDR *highpc, int need_pc, struct dwarf2_cu *cu)
+		      CORE_ADDR *highpc, int set_addrmap,
+		      struct dwarf2_cu *cu)
 {
   struct partial_die_info *pdi;
 
@@ -6575,7 +6575,7 @@ scan_partial_symbols (struct partial_die_info *first_die, CORE_ADDR *lowpc,
 	  switch (pdi->tag)
 	    {
 	    case DW_TAG_subprogram:
-	      add_partial_subprogram (pdi, lowpc, highpc, need_pc, cu);
+	      add_partial_subprogram (pdi, lowpc, highpc, set_addrmap, cu);
 	      break;
 	    case DW_TAG_constant:
 	    case DW_TAG_variable:
@@ -6605,10 +6605,10 @@ scan_partial_symbols (struct partial_die_info *first_die, CORE_ADDR *lowpc,
 	      add_partial_symbol (pdi, cu);
 	      break;
 	    case DW_TAG_namespace:
-	      add_partial_namespace (pdi, lowpc, highpc, need_pc, cu);
+	      add_partial_namespace (pdi, lowpc, highpc, set_addrmap, cu);
 	      break;
 	    case DW_TAG_module:
-	      add_partial_module (pdi, lowpc, highpc, need_pc, cu);
+	      add_partial_module (pdi, lowpc, highpc, set_addrmap, cu);
 	      break;
 	    case DW_TAG_imported_unit:
 	      {
@@ -6975,7 +6975,7 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
 static void
 add_partial_namespace (struct partial_die_info *pdi,
 		       CORE_ADDR *lowpc, CORE_ADDR *highpc,
-		       int need_pc, struct dwarf2_cu *cu)
+		       int set_addrmap, struct dwarf2_cu *cu)
 {
   /* Add a symbol for the namespace.  */
 
@@ -6984,14 +6984,14 @@ add_partial_namespace (struct partial_die_info *pdi,
   /* Now scan partial symbols in that namespace.  */
 
   if (pdi->has_children)
-    scan_partial_symbols (pdi->die_child, lowpc, highpc, need_pc, cu);
+    scan_partial_symbols (pdi->die_child, lowpc, highpc, set_addrmap, cu);
 }
 
 /* Read a partial die corresponding to a Fortran module.  */
 
 static void
 add_partial_module (struct partial_die_info *pdi, CORE_ADDR *lowpc,
-		    CORE_ADDR *highpc, int need_pc, struct dwarf2_cu *cu)
+		    CORE_ADDR *highpc, int set_addrmap, struct dwarf2_cu *cu)
 {
   /* Add a symbol for the namespace.  */
 
@@ -7000,15 +7000,17 @@ add_partial_module (struct partial_die_info *pdi, CORE_ADDR *lowpc,
   /* Now scan partial symbols in that module.  */
 
   if (pdi->has_children)
-    scan_partial_symbols (pdi->die_child, lowpc, highpc, need_pc, cu);
+    scan_partial_symbols (pdi->die_child, lowpc, highpc, set_addrmap, cu);
 }
 
 /* Read a partial die corresponding to a subprogram and create a partial
    symbol for that subprogram.  When the CU language allows it, this
    routine also defines a partial symbol for each nested subprogram
-   that this subprogram contains.
+   that this subprogram contains.  If SET_ADDRMAP is true, record the
+   covered ranges in the addrmap.  *LOWPC and *HIGHPC to the lowest and
+   highest PC values found in PDI.
 
-   DIE my also be a lexical block, in which case we simply search
+   PDI may also be a lexical block, in which case we simply search
    recursively for suprograms defined inside that lexical block.
    Again, this is only performed when the CU language allows this
    type of definitions.  */
@@ -7016,7 +7018,7 @@ add_partial_module (struct partial_die_info *pdi, CORE_ADDR *lowpc,
 static void
 add_partial_subprogram (struct partial_die_info *pdi,
 			CORE_ADDR *lowpc, CORE_ADDR *highpc,
-			int need_pc, struct dwarf2_cu *cu)
+			int set_addrmap, struct dwarf2_cu *cu)
 {
   if (pdi->tag == DW_TAG_subprogram)
     {
@@ -7026,7 +7028,7 @@ add_partial_subprogram (struct partial_die_info *pdi,
             *lowpc = pdi->lowpc;
           if (pdi->highpc > *highpc)
             *highpc = pdi->highpc;
-	  if (need_pc)
+	  if (set_addrmap)
 	    {
 	      CORE_ADDR baseaddr;
 	      struct objfile *objfile = cu->objfile;
@@ -7062,7 +7064,7 @@ add_partial_subprogram (struct partial_die_info *pdi,
 	  fixup_partial_die (pdi, cu);
 	  if (pdi->tag == DW_TAG_subprogram
 	      || pdi->tag == DW_TAG_lexical_block)
-	    add_partial_subprogram (pdi, lowpc, highpc, need_pc, cu);
+	    add_partial_subprogram (pdi, lowpc, highpc, set_addrmap, cu);
 	  pdi = pdi->die_sibling;
 	}
     }
-- 
1.9.3

Re: [PATCH] Update comments in scan_partial_symbols and add_partial_subprogram

[Pedro Alves]

Hi Yao,

On 08/22/2014 01:02 PM, Yao Qi wrote:

>  /* Read a partial die corresponding to a subprogram and create a partial
>     symbol for that subprogram.  When the CU language allows it, this
>     routine also defines a partial symbol for each nested subprogram
> -   that this subprogram contains.
> +   that this subprogram contains.  If SET_ADDRMAP is true, record the
> +   covered ranges in the addrmap.  *LOWPC and *HIGHPC to the lowest and
> +   highest PC values found in PDI.

The last sentence is missing the verb?

> -   DIE my also be a lexical block, in which case we simply search
> +   PDI may also be a lexical block, in which case we simply search
>     recursively for suprograms defined inside that lexical block.

As long as we're fixing typos, there's another here "suprograms".

Otherwise looks fine to me.

Thanks,
Pedro Alves

Re: [PATCH] Update comments in scan_partial_symbols and add_partial_subprogram

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

>> -   that this subprogram contains.
>> +   that this subprogram contains.  If SET_ADDRMAP is true, record the
>> +   covered ranges in the addrmap.  *LOWPC and *HIGHPC to the lowest and
>> +   highest PC values found in PDI.
>
> The last sentence is missing the verb?

Ur, "are" is missing, I think.  It should be "... are found in PDI.".
Fixed in the updated patch.

>
>> -   DIE my also be a lexical block, in which case we simply search
>> +   PDI may also be a lexical block, in which case we simply search
>>     recursively for suprograms defined inside that lexical block.
>
> As long as we're fixing typos, there's another here "suprograms".

Fixed.

>
> Otherwise looks fine to me.

Patch below is pushed in.

-- 
Yao (齐尧)


commit cdc07690a4812fb41d3766d087cf7ec78184d6a8
Author: Yao Qi <yao@codesourcery.com>
Date:   Fri Aug 15 19:06:02 2014 +0800

    Update comments in scan_partial_symbols and add_partial_subprogram
    
    I read comment of scan_partial_symbols about NEED_PC and how *LOWPC
    and *HIGHPC are updated:
    
       DW_AT_ranges).  If NEED_PC is set, then this function will set
       *LOWPC and *HIGHPC to the lowest and highest PC values found in CU
       and record the covered ranges in the addrmap.
    
    NEED_PC is only used in the callee of scan_partial_symbols,
    add_partial_subprogram,
    
      if (pdi->tag == DW_TAG_subprogram)
        {
          if (pdi->has_pc_info)
            {
              if (pdi->lowpc < *lowpc)
                *lowpc = pdi->lowpc;
              if (pdi->highpc > *highpc)
                *highpc = pdi->highpc;
    	  if (need_pc)
    
    *LOWPC and *HIGHPC is updated regardless of NEED_PC.  When NEED_PC is
    true, addrmap is updated.  It would be clear to rename NEED_PC to
    SET_ADDRMAP.  That is what this patch does.  Beside this, this patch
    also adjust comments in related functions.
    
    gdb:
    
    2014-08-24  Yao Qi  <yao@codesourcery.com>
    
    	* dwarf2read.c (scan_partial_symbols):  Update comments.
    	Rename argument 'need_pc' with 'set_addrmap'.
    	（add_partial_namespace): Rename argument 'need_pc' with
    	'set_addrmap'.
    	(add_partial_module): Likewise.
    	(add_partial_subprogram): Likewise.  Update comments.
    	(dwarf2_name): Fix typo.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0978d63..1660577 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2014-08-24  Yao Qi  <yao@codesourcery.com>
+
+	* dwarf2read.c (scan_partial_symbols):  Update comments.
+	Rename argument 'need_pc' with 'set_addrmap'.
+	（add_partial_namespace): Rename argument 'need_pc' with
+	'set_addrmap'.
+	(add_partial_module): Likewise.
+	(add_partial_subprogram): Likewise.  Update comments.
+	(dwarf2_name): Fix typo.
+
 2014-08-22  Doug Evans  <dje@google.com>
 
 	PR 17276
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 458977e..df6deef 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1387,10 +1387,10 @@ static void add_partial_symbol (struct partial_die_info *,
 
 static void add_partial_namespace (struct partial_die_info *pdi,
 				   CORE_ADDR *lowpc, CORE_ADDR *highpc,
-				   int need_pc, struct dwarf2_cu *cu);
+				   int set_addrmap, struct dwarf2_cu *cu);
 
 static void add_partial_module (struct partial_die_info *pdi, CORE_ADDR *lowpc,
-				CORE_ADDR *highpc, int need_pc,
+				CORE_ADDR *highpc, int set_addrmap,
 				struct dwarf2_cu *cu);
 
 static void add_partial_enumeration (struct partial_die_info *enum_pdi,
@@ -6542,15 +6542,15 @@ create_all_comp_units (struct objfile *objfile)
 }
 
 /* Process all loaded DIEs for compilation unit CU, starting at
-   FIRST_DIE.  The caller should pass NEED_PC == 1 if the compilation
+   FIRST_DIE.  The caller should pass SET_ADDRMAP == 1 if the compilation
    unit DIE did not have PC info (DW_AT_low_pc and DW_AT_high_pc, or
-   DW_AT_ranges).  If NEED_PC is set, then this function will set
-   *LOWPC and *HIGHPC to the lowest and highest PC values found in CU
-   and record the covered ranges in the addrmap.  */
+   DW_AT_ranges).  See the comments of add_partial_subprogram on how
+   SET_ADDRMAP is used and how *LOWPC and *HIGHPC are updated.  */
 
 static void
 scan_partial_symbols (struct partial_die_info *first_die, CORE_ADDR *lowpc,
-		      CORE_ADDR *highpc, int need_pc, struct dwarf2_cu *cu)
+		      CORE_ADDR *highpc, int set_addrmap,
+		      struct dwarf2_cu *cu)
 {
   struct partial_die_info *pdi;
 
@@ -6575,7 +6575,7 @@ scan_partial_symbols (struct partial_die_info *first_die, CORE_ADDR *lowpc,
 	  switch (pdi->tag)
 	    {
 	    case DW_TAG_subprogram:
-	      add_partial_subprogram (pdi, lowpc, highpc, need_pc, cu);
+	      add_partial_subprogram (pdi, lowpc, highpc, set_addrmap, cu);
 	      break;
 	    case DW_TAG_constant:
 	    case DW_TAG_variable:
@@ -6605,10 +6605,10 @@ scan_partial_symbols (struct partial_die_info *first_die, CORE_ADDR *lowpc,
 	      add_partial_symbol (pdi, cu);
 	      break;
 	    case DW_TAG_namespace:
-	      add_partial_namespace (pdi, lowpc, highpc, need_pc, cu);
+	      add_partial_namespace (pdi, lowpc, highpc, set_addrmap, cu);
 	      break;
 	    case DW_TAG_module:
-	      add_partial_module (pdi, lowpc, highpc, need_pc, cu);
+	      add_partial_module (pdi, lowpc, highpc, set_addrmap, cu);
 	      break;
 	    case DW_TAG_imported_unit:
 	      {
@@ -6975,7 +6975,7 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
 static void
 add_partial_namespace (struct partial_die_info *pdi,
 		       CORE_ADDR *lowpc, CORE_ADDR *highpc,
-		       int need_pc, struct dwarf2_cu *cu)
+		       int set_addrmap, struct dwarf2_cu *cu)
 {
   /* Add a symbol for the namespace.  */
 
@@ -6984,14 +6984,14 @@ add_partial_namespace (struct partial_die_info *pdi,
   /* Now scan partial symbols in that namespace.  */
 
   if (pdi->has_children)
-    scan_partial_symbols (pdi->die_child, lowpc, highpc, need_pc, cu);
+    scan_partial_symbols (pdi->die_child, lowpc, highpc, set_addrmap, cu);
 }
 
 /* Read a partial die corresponding to a Fortran module.  */
 
 static void
 add_partial_module (struct partial_die_info *pdi, CORE_ADDR *lowpc,
-		    CORE_ADDR *highpc, int need_pc, struct dwarf2_cu *cu)
+		    CORE_ADDR *highpc, int set_addrmap, struct dwarf2_cu *cu)
 {
   /* Add a symbol for the namespace.  */
 
@@ -7000,23 +7000,25 @@ add_partial_module (struct partial_die_info *pdi, CORE_ADDR *lowpc,
   /* Now scan partial symbols in that module.  */
 
   if (pdi->has_children)
-    scan_partial_symbols (pdi->die_child, lowpc, highpc, need_pc, cu);
+    scan_partial_symbols (pdi->die_child, lowpc, highpc, set_addrmap, cu);
 }
 
 /* Read a partial die corresponding to a subprogram and create a partial
    symbol for that subprogram.  When the CU language allows it, this
    routine also defines a partial symbol for each nested subprogram
-   that this subprogram contains.
+   that this subprogram contains.  If SET_ADDRMAP is true, record the
+   covered ranges in the addrmap.  *LOWPC and *HIGHPC to the lowest and
+   highest PC values are found in PDI.
 
-   DIE my also be a lexical block, in which case we simply search
-   recursively for suprograms defined inside that lexical block.
+   PDI may also be a lexical block, in which case we simply search
+   recursively for subprograms defined inside that lexical block.
    Again, this is only performed when the CU language allows this
    type of definitions.  */
 
 static void
 add_partial_subprogram (struct partial_die_info *pdi,
 			CORE_ADDR *lowpc, CORE_ADDR *highpc,
-			int need_pc, struct dwarf2_cu *cu)
+			int set_addrmap, struct dwarf2_cu *cu)
 {
   if (pdi->tag == DW_TAG_subprogram)
     {
@@ -7026,7 +7028,7 @@ add_partial_subprogram (struct partial_die_info *pdi,
             *lowpc = pdi->lowpc;
           if (pdi->highpc > *highpc)
             *highpc = pdi->highpc;
-	  if (need_pc)
+	  if (set_addrmap)
 	    {
 	      CORE_ADDR baseaddr;
 	      struct objfile *objfile = cu->objfile;
@@ -7062,7 +7064,7 @@ add_partial_subprogram (struct partial_die_info *pdi,
 	  fixup_partial_die (pdi, cu);
 	  if (pdi->tag == DW_TAG_subprogram
 	      || pdi->tag == DW_TAG_lexical_block)
-	    add_partial_subprogram (pdi, lowpc, highpc, need_pc, cu);
+	    add_partial_subprogram (pdi, lowpc, highpc, set_addrmap, cu);
 	  pdi = pdi->die_sibling;
 	}
     }
@@ -19025,7 +19027,7 @@ dwarf2_name (struct die_info *die, struct dwarf2_cu *cu)
 	  /* GCJ will output '<init>' for Java constructor names.
 	     For this special case, return the name of the parent class.  */
 
-	  /* GCJ may output suprogram DIEs with AT_specification set.
+	  /* GCJ may output subprogram DIEs with AT_specification set.
 	     If so, use the name of the specified DIE.  */
 	  spec_die = die_specification (die, &spec_cu);
 	  if (spec_die != NULL)

Re: [PATCH] Update comments in scan_partial_symbols and add_partial_subprogram

[Pedro Alves]

On 08/24/2014 02:48 PM, Yao Qi wrote:
>>> >> -   that this subprogram contains.
>>> >> +   that this subprogram contains.  If SET_ADDRMAP is true, record the
>>> >> +   covered ranges in the addrmap.  *LOWPC and *HIGHPC to the lowest and
>>> >> +   highest PC values found in PDI.
>> >
>> > The last sentence is missing the verb?
> Ur, "are" is missing, I think.  It should be "... are found in PDI.".
> Fixed in the updated patch.

...

> +   that this subprogram contains.  If SET_ADDRMAP is true, record the
> +   covered ranges in the addrmap.  *LOWPC and *HIGHPC to the lowest and
> +   highest PC values are found in PDI.

Hmm.  I still can't parse this though.  It's actually the "to the" part
that seems to miss a verb to me.  Isn't a "Set" missing instead of
"are"?  Like:

  that this subprogram contains.  If SET_ADDRMAP is true, record the
  covered ranges in the addrmap.  Set *LOWPC and *HIGHPC to the lowest and
  highest PC values found in PDI.

Thanks,
Pedro Alves

Re: [PATCH] Update comments in scan_partial_symbols and add_partial_subprogram

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> Hmm.  I still can't parse this though.  It's actually the "to the" part
> that seems to miss a verb to me.  Isn't a "Set" missing instead of
> "are"?  Like:
>
>   that this subprogram contains.  If SET_ADDRMAP is true, record the
>   covered ranges in the addrmap.  Set *LOWPC and *HIGHPC to the lowest and
>   highest PC values found in PDI.

Sigh, my brain stopped working!  Sorry about that.  Fix it as you suggested.

-- 
Yao (齐尧)

commit 9afcfde91565f7fa16e77794cef0a05ebe77bf1b
Author: Yao Qi <yao@codesourcery.com>
Date:   Mon Aug 25 20:01:45 2014 +0800

    Fix grammatical error in comments
    
    gdb:
    
    2014-08-25  Yao Qi  <yao@codesourcery.com>
    
    	* dwarf2read.c: Fix grammatical error.

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index df6deef..4720198 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -7007,8 +7007,8 @@ add_partial_module (struct partial_die_info *pdi, CORE_ADDR *lowpc,
    symbol for that subprogram.  When the CU language allows it, this
    routine also defines a partial symbol for each nested subprogram
    that this subprogram contains.  If SET_ADDRMAP is true, record the
-   covered ranges in the addrmap.  *LOWPC and *HIGHPC to the lowest and
-   highest PC values are found in PDI.
+   covered ranges in the addrmap.  Set *LOWPC and *HIGHPC to the lowest
+   and highest PC values found in PDI.
 
    PDI may also be a lexical block, in which case we simply search
    recursively for subprograms defined inside that lexical block.