[PATCH] gdb: Fix issue with Clang CLI macros

[PATCH] gdb: Fix issue with Clang CLI macros

[Bruno Larsen]

Clang up to the current version adds macros that were defined in the
command line or by "other means", according to the Dwarf specification,
after the last DW_MACRO_end_file, instead of before the first
DW_MACRO_start_file, as the specification dictates. This has been
submitted as a bug to Clang developers, but seeing as there is no
expected date for it to be fixed, a workaround was added for all current
versions of Clang.

The workaround detects when the main file would be closed and if the
producer is clang, and turns that operation into a noop, so we keep a
reference to the current_file as those macros are read.

This patch fixes PR macros/29034, and can be tested by running
gdb.base/macscp.exp using clang, the test printing FROM_COMMANDLINE
should be fixed.
---
 gdb/dwarf2/cu.h    |  1 +
 gdb/dwarf2/macro.c | 17 +++++++++++++----
 gdb/dwarf2/macro.h |  2 +-
 gdb/dwarf2/read.c  | 15 ++++++++++++++-
 gdb/dwarf2/read.h  |  2 ++
 gdb/producer.c     | 26 ++++++++++++++++++++++++++
 gdb/producer.h     |  4 ++++
 7 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/gdb/dwarf2/cu.h b/gdb/dwarf2/cu.h
index 6b72ec234bf..3b7f92be406 100644
--- a/gdb/dwarf2/cu.h
+++ b/gdb/dwarf2/cu.h
@@ -257,6 +257,7 @@ struct dwarf2_cu
   bool producer_is_icc : 1;
   bool producer_is_icc_lt_14 : 1;
   bool producer_is_codewarrior : 1;
+  bool producer_is_clang_le_14 : 1;
 
   /* When true, the file that we're processing is known to have
      debugging info for C++ namespaces.  GCC 3.3.x did not produce
diff --git a/gdb/dwarf2/macro.c b/gdb/dwarf2/macro.c
index 99c3653a2c3..305af1a7f47 100644
--- a/gdb/dwarf2/macro.c
+++ b/gdb/dwarf2/macro.c
@@ -431,7 +431,7 @@ dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,
 			  struct dwarf2_section_info *str_section,
 			  struct dwarf2_section_info *str_offsets_section,
 			  gdb::optional<ULONGEST> str_offsets_base,
-			  htab_t include_hash)
+			  htab_t include_hash, struct dwarf2_cu *cu)
 {
   struct objfile *objfile = per_objfile->objfile;
   enum dwarf_macro_record_type macinfo_type;
@@ -658,6 +658,15 @@ dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,
 	  if (! current_file)
 	    complaint (_("macro debug info has an unmatched "
 			 "`close_file' directive"));
+	  else if (current_file->included_by == nullptr
+		   && producer_is_clang_le_14 (cu))
+	    {
+	      /* Clang, until the current version, misplaces macro definitions,
+		 putting them after the last DW_MACRO_end_file instead of
+		 before the first DW_MACRO_start_file.  This condition should be
+		 removed once we can reasonably expect most clang users to have
+		 updated to a fixed version.  */
+	    }
 	  else
 	    {
 	      current_file = current_file->included_by;
@@ -737,7 +746,7 @@ dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,
 					  current_file, lh, section,
 					  section_is_gnu, is_dwz, offset_size,
 					  str_section, str_offsets_section,
-					  str_offsets_base, include_hash);
+					  str_offsets_base, include_hash, cu);
 
 		htab_remove_elt (include_hash, (void *) new_mac_ptr);
 	      }
@@ -781,7 +790,7 @@ dwarf_decode_macros (dwarf2_per_objfile *per_objfile,
 		     unsigned int offset, struct dwarf2_section_info *str_section,
 		     struct dwarf2_section_info *str_offsets_section,
 		     gdb::optional<ULONGEST> str_offsets_base,
-		     int section_is_gnu)
+		     int section_is_gnu, struct dwarf2_cu *cu)
 {
   bfd *abfd;
   const gdb_byte *mac_ptr, *mac_end;
@@ -942,5 +951,5 @@ dwarf_decode_macros (dwarf2_per_objfile *per_objfile,
   dwarf_decode_macro_bytes (per_objfile, builder, abfd, mac_ptr, mac_end,
 			    current_file, lh, section, section_is_gnu, 0,
 			    offset_size, str_section, str_offsets_section,
-			    str_offsets_base, include_hash.get ());
+			    str_offsets_base, include_hash.get (), cu);
 }
diff --git a/gdb/dwarf2/macro.h b/gdb/dwarf2/macro.h
index e8c33a34a8d..02753ef377a 100644
--- a/gdb/dwarf2/macro.h
+++ b/gdb/dwarf2/macro.h
@@ -31,6 +31,6 @@ extern void dwarf_decode_macros (dwarf2_per_objfile *per_objfile,
 				 dwarf2_section_info *str_section,
 				 dwarf2_section_info *str_offsets_section,
 				 gdb::optional<ULONGEST> str_offsets_base,
-				 int section_is_gnu);
+				 int section_is_gnu, struct dwarf2_cu *cu);
 
 #endif /* GDB_DWARF2_MACRO_H */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 698720276a9..1fc5d5d3dd8 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -9454,6 +9454,15 @@ producer_is_gcc_lt_4_3 (struct dwarf2_cu *cu)
   return cu->producer_is_gcc_lt_4_3;
 }
 
+bool
+producer_is_clang_le_14 (struct dwarf2_cu *cu)
+{
+  if (!cu->checked_producer)
+    check_producer (cu);
+  
+  return cu->producer_is_clang_le_14;
+}
+
 static file_and_directory &
 find_file_and_directory (struct die_info *die, struct dwarf2_cu *cu)
 {
@@ -13295,6 +13304,10 @@ check_producer (struct dwarf2_cu *cu)
     }
   else if (startswith (cu->producer, "CodeWarrior S12/L-ISA"))
     cu->producer_is_codewarrior = true;
+  else if (producer_is_clang (cu->producer, &major, &minor))
+    {
+      cu->producer_is_clang_le_14 = major <= 14;
+    }
   else
     {
       /* For other non-GCC compilers, expect their behavior is DWARF version
@@ -23218,7 +23231,7 @@ dwarf_decode_macros (struct dwarf2_cu *cu, unsigned int offset,
 
   dwarf_decode_macros (per_objfile, builder, section, lh,
 		       offset_size, offset, str_section, str_offsets_section,
-		       str_offsets_base, section_is_gnu);
+		       str_offsets_base, section_is_gnu, cu);
 }
 
 /* Return the .debug_loc section to use for CU.
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index be3b9d19601..23f040a6ca8 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -677,4 +677,6 @@ extern void dwarf2_get_section_info (struct objfile *,
 				     asection **, const gdb_byte **,
 				     bfd_size_type *);
 
+extern bool producer_is_clang_le_14(struct dwarf2_cu *cu);
+
 #endif /* DWARF2READ_H */
diff --git a/gdb/producer.c b/gdb/producer.c
index ef1dd93afbc..a9fb7dd8245 100644
--- a/gdb/producer.c
+++ b/gdb/producer.c
@@ -127,6 +127,32 @@ producer_is_llvm (const char *producer)
 				 || startswith (producer, " F90 Flang ")));
 }
 
+/* see producer.h */
+
+bool
+producer_is_clang (const char* producer, int *major, int* minor)
+{
+  const char *cs;
+
+  if (producer != nullptr && startswith (producer, "clang "))
+    {
+      int maj, min;
+      if (major == nullptr)
+	major = &maj;
+      if (minor == nullptr)
+	minor = &min;
+
+      /* The full producer sting will look something like
+	 "clang version XX.X.X ..."
+	 So we can safely ignore all characters before the first digit.  */
+      cs = producer + strlen("clang version ");
+
+      if (sscanf (cs, "%d.%d", major, minor) == 2)
+	  return true;
+    }
+  return false;
+}
+
 #if defined GDB_SELF_TEST
 namespace selftests {
 namespace producer {
diff --git a/gdb/producer.h b/gdb/producer.h
index f7c19368bc6..b75cfae6569 100644
--- a/gdb/producer.h
+++ b/gdb/producer.h
@@ -41,4 +41,8 @@ extern bool producer_is_icc (const char *producer, int *major, int *minor);
    false otherwise.*/
 extern bool producer_is_llvm (const char *producer);
 
+/* Returns true if the given PRODUCER string is clang, false otherwise.
+   Sets MAJOR and MINOR accordingly, if not NULL.  */
+extern bool producer_is_clang (const char *producer, int *major, int *minor);
+
 #endif
-- 
2.31.1

[PING] [PATCH] gdb: Fix issue with Clang CLI macros

[Bruno Larsen]

ping

Cheers!
Bruno Larsen

On 4/20/22 14:41, Bruno Larsen wrote:
> Clang up to the current version adds macros that were defined in the
> command line or by "other means", according to the Dwarf specification,
> after the last DW_MACRO_end_file, instead of before the first
> DW_MACRO_start_file, as the specification dictates. This has been
> submitted as a bug to Clang developers, but seeing as there is no
> expected date for it to be fixed, a workaround was added for all current
> versions of Clang.
> 
> The workaround detects when the main file would be closed and if the
> producer is clang, and turns that operation into a noop, so we keep a
> reference to the current_file as those macros are read.
> 
> This patch fixes PR macros/29034, and can be tested by running
> gdb.base/macscp.exp using clang, the test printing FROM_COMMANDLINE
> should be fixed.
> ---
>   gdb/dwarf2/cu.h    |  1 +
>   gdb/dwarf2/macro.c | 17 +++++++++++++----
>   gdb/dwarf2/macro.h |  2 +-
>   gdb/dwarf2/read.c  | 15 ++++++++++++++-
>   gdb/dwarf2/read.h  |  2 ++
>   gdb/producer.c     | 26 ++++++++++++++++++++++++++
>   gdb/producer.h     |  4 ++++
>   7 files changed, 61 insertions(+), 6 deletions(-)
> 
> diff --git a/gdb/dwarf2/cu.h b/gdb/dwarf2/cu.h
> index 6b72ec234bf..3b7f92be406 100644
> --- a/gdb/dwarf2/cu.h
> +++ b/gdb/dwarf2/cu.h
> @@ -257,6 +257,7 @@ struct dwarf2_cu
>     bool producer_is_icc : 1;
>     bool producer_is_icc_lt_14 : 1;
>     bool producer_is_codewarrior : 1;
> +  bool producer_is_clang_le_14 : 1;
>   
>     /* When true, the file that we're processing is known to have
>        debugging info for C++ namespaces.  GCC 3.3.x did not produce
> diff --git a/gdb/dwarf2/macro.c b/gdb/dwarf2/macro.c
> index 99c3653a2c3..305af1a7f47 100644
> --- a/gdb/dwarf2/macro.c
> +++ b/gdb/dwarf2/macro.c
> @@ -431,7 +431,7 @@ dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,
>   			  struct dwarf2_section_info *str_section,
>   			  struct dwarf2_section_info *str_offsets_section,
>   			  gdb::optional<ULONGEST> str_offsets_base,
> -			  htab_t include_hash)
> +			  htab_t include_hash, struct dwarf2_cu *cu)
>   {
>     struct objfile *objfile = per_objfile->objfile;
>     enum dwarf_macro_record_type macinfo_type;
> @@ -658,6 +658,15 @@ dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,
>   	  if (! current_file)
>   	    complaint (_("macro debug info has an unmatched "
>   			 "`close_file' directive"));
> +	  else if (current_file->included_by == nullptr
> +		   && producer_is_clang_le_14 (cu))
> +	    {
> +	      /* Clang, until the current version, misplaces macro definitions,
> +		 putting them after the last DW_MACRO_end_file instead of
> +		 before the first DW_MACRO_start_file.  This condition should be
> +		 removed once we can reasonably expect most clang users to have
> +		 updated to a fixed version.  */
> +	    }
>   	  else
>   	    {
>   	      current_file = current_file->included_by;
> @@ -737,7 +746,7 @@ dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,
>   					  current_file, lh, section,
>   					  section_is_gnu, is_dwz, offset_size,
>   					  str_section, str_offsets_section,
> -					  str_offsets_base, include_hash);
> +					  str_offsets_base, include_hash, cu);
>   
>   		htab_remove_elt (include_hash, (void *) new_mac_ptr);
>   	      }
> @@ -781,7 +790,7 @@ dwarf_decode_macros (dwarf2_per_objfile *per_objfile,
>   		     unsigned int offset, struct dwarf2_section_info *str_section,
>   		     struct dwarf2_section_info *str_offsets_section,
>   		     gdb::optional<ULONGEST> str_offsets_base,
> -		     int section_is_gnu)
> +		     int section_is_gnu, struct dwarf2_cu *cu)
>   {
>     bfd *abfd;
>     const gdb_byte *mac_ptr, *mac_end;
> @@ -942,5 +951,5 @@ dwarf_decode_macros (dwarf2_per_objfile *per_objfile,
>     dwarf_decode_macro_bytes (per_objfile, builder, abfd, mac_ptr, mac_end,
>   			    current_file, lh, section, section_is_gnu, 0,
>   			    offset_size, str_section, str_offsets_section,
> -			    str_offsets_base, include_hash.get ());
> +			    str_offsets_base, include_hash.get (), cu);
>   }
> diff --git a/gdb/dwarf2/macro.h b/gdb/dwarf2/macro.h
> index e8c33a34a8d..02753ef377a 100644
> --- a/gdb/dwarf2/macro.h
> +++ b/gdb/dwarf2/macro.h
> @@ -31,6 +31,6 @@ extern void dwarf_decode_macros (dwarf2_per_objfile *per_objfile,
>   				 dwarf2_section_info *str_section,
>   				 dwarf2_section_info *str_offsets_section,
>   				 gdb::optional<ULONGEST> str_offsets_base,
> -				 int section_is_gnu);
> +				 int section_is_gnu, struct dwarf2_cu *cu);
>   
>   #endif /* GDB_DWARF2_MACRO_H */
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 698720276a9..1fc5d5d3dd8 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -9454,6 +9454,15 @@ producer_is_gcc_lt_4_3 (struct dwarf2_cu *cu)
>     return cu->producer_is_gcc_lt_4_3;
>   }
>   
> +bool
> +producer_is_clang_le_14 (struct dwarf2_cu *cu)
> +{
> +  if (!cu->checked_producer)
> +    check_producer (cu);
> +
> +  return cu->producer_is_clang_le_14;
> +}
> +
>   static file_and_directory &
>   find_file_and_directory (struct die_info *die, struct dwarf2_cu *cu)
>   {
> @@ -13295,6 +13304,10 @@ check_producer (struct dwarf2_cu *cu)
>       }
>     else if (startswith (cu->producer, "CodeWarrior S12/L-ISA"))
>       cu->producer_is_codewarrior = true;
> +  else if (producer_is_clang (cu->producer, &major, &minor))
> +    {
> +      cu->producer_is_clang_le_14 = major <= 14;
> +    }
>     else
>       {
>         /* For other non-GCC compilers, expect their behavior is DWARF version
> @@ -23218,7 +23231,7 @@ dwarf_decode_macros (struct dwarf2_cu *cu, unsigned int offset,
>   
>     dwarf_decode_macros (per_objfile, builder, section, lh,
>   		       offset_size, offset, str_section, str_offsets_section,
> -		       str_offsets_base, section_is_gnu);
> +		       str_offsets_base, section_is_gnu, cu);
>   }
>   
>   /* Return the .debug_loc section to use for CU.
> diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
> index be3b9d19601..23f040a6ca8 100644
> --- a/gdb/dwarf2/read.h
> +++ b/gdb/dwarf2/read.h
> @@ -677,4 +677,6 @@ extern void dwarf2_get_section_info (struct objfile *,
>   				     asection **, const gdb_byte **,
>   				     bfd_size_type *);
>   
> +extern bool producer_is_clang_le_14(struct dwarf2_cu *cu);
> +
>   #endif /* DWARF2READ_H */
> diff --git a/gdb/producer.c b/gdb/producer.c
> index ef1dd93afbc..a9fb7dd8245 100644
> --- a/gdb/producer.c
> +++ b/gdb/producer.c
> @@ -127,6 +127,32 @@ producer_is_llvm (const char *producer)
>   				 || startswith (producer, " F90 Flang ")));
>   }
>   
> +/* see producer.h */
> +
> +bool
> +producer_is_clang (const char* producer, int *major, int* minor)
> +{
> +  const char *cs;
> +
> +  if (producer != nullptr && startswith (producer, "clang "))
> +    {
> +      int maj, min;
> +      if (major == nullptr)
> +	major = &maj;
> +      if (minor == nullptr)
> +	minor = &min;
> +
> +      /* The full producer sting will look something like
> +	 "clang version XX.X.X ..."
> +	 So we can safely ignore all characters before the first digit.  */
> +      cs = producer + strlen("clang version ");
> +
> +      if (sscanf (cs, "%d.%d", major, minor) == 2)
> +	  return true;
> +    }
> +  return false;
> +}
> +
>   #if defined GDB_SELF_TEST
>   namespace selftests {
>   namespace producer {
> diff --git a/gdb/producer.h b/gdb/producer.h
> index f7c19368bc6..b75cfae6569 100644
> --- a/gdb/producer.h
> +++ b/gdb/producer.h
> @@ -41,4 +41,8 @@ extern bool producer_is_icc (const char *producer, int *major, int *minor);
>      false otherwise.*/
>   extern bool producer_is_llvm (const char *producer);
>   
> +/* Returns true if the given PRODUCER string is clang, false otherwise.
> +   Sets MAJOR and MINOR accordingly, if not NULL.  */
> +extern bool producer_is_clang (const char *producer, int *major, int *minor);
> +
>   #endif

Re: [PING] [PATCH] gdb: Fix issue with Clang CLI macros

[Bruno Larsen]

ping!

Cheers!
Bruno Larsen

On 5/4/22 09:50, Bruno Larsen wrote:
> ping
> 
> Cheers!
> Bruno Larsen
> 
> On 4/20/22 14:41, Bruno Larsen wrote:
>> Clang up to the current version adds macros that were defined in the
>> command line or by "other means", according to the Dwarf specification,
>> after the last DW_MACRO_end_file, instead of before the first
>> DW_MACRO_start_file, as the specification dictates. This has been
>> submitted as a bug to Clang developers, but seeing as there is no
>> expected date for it to be fixed, a workaround was added for all current
>> versions of Clang.
>>
>> The workaround detects when the main file would be closed and if the
>> producer is clang, and turns that operation into a noop, so we keep a
>> reference to the current_file as those macros are read.
>>
>> This patch fixes PR macros/29034, and can be tested by running
>> gdb.base/macscp.exp using clang, the test printing FROM_COMMANDLINE
>> should be fixed.
>> ---
>>   gdb/dwarf2/cu.h    |  1 +
>>   gdb/dwarf2/macro.c | 17 +++++++++++++----
>>   gdb/dwarf2/macro.h |  2 +-
>>   gdb/dwarf2/read.c  | 15 ++++++++++++++-
>>   gdb/dwarf2/read.h  |  2 ++
>>   gdb/producer.c     | 26 ++++++++++++++++++++++++++
>>   gdb/producer.h     |  4 ++++
>>   7 files changed, 61 insertions(+), 6 deletions(-)
>>
>> diff --git a/gdb/dwarf2/cu.h b/gdb/dwarf2/cu.h
>> index 6b72ec234bf..3b7f92be406 100644
>> --- a/gdb/dwarf2/cu.h
>> +++ b/gdb/dwarf2/cu.h
>> @@ -257,6 +257,7 @@ struct dwarf2_cu
>>     bool producer_is_icc : 1;
>>     bool producer_is_icc_lt_14 : 1;
>>     bool producer_is_codewarrior : 1;
>> +  bool producer_is_clang_le_14 : 1;
>>     /* When true, the file that we're processing is known to have
>>        debugging info for C++ namespaces.  GCC 3.3.x did not produce
>> diff --git a/gdb/dwarf2/macro.c b/gdb/dwarf2/macro.c
>> index 99c3653a2c3..305af1a7f47 100644
>> --- a/gdb/dwarf2/macro.c
>> +++ b/gdb/dwarf2/macro.c
>> @@ -431,7 +431,7 @@ dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,
>>                 struct dwarf2_section_info *str_section,
>>                 struct dwarf2_section_info *str_offsets_section,
>>                 gdb::optional<ULONGEST> str_offsets_base,
>> -              htab_t include_hash)
>> +              htab_t include_hash, struct dwarf2_cu *cu)
>>   {
>>     struct objfile *objfile = per_objfile->objfile;
>>     enum dwarf_macro_record_type macinfo_type;
>> @@ -658,6 +658,15 @@ dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,
>>         if (! current_file)
>>           complaint (_("macro debug info has an unmatched "
>>                "`close_file' directive"));
>> +      else if (current_file->included_by == nullptr
>> +           && producer_is_clang_le_14 (cu))
>> +        {
>> +          /* Clang, until the current version, misplaces macro definitions,
>> +         putting them after the last DW_MACRO_end_file instead of
>> +         before the first DW_MACRO_start_file.  This condition should be
>> +         removed once we can reasonably expect most clang users to have
>> +         updated to a fixed version.  */
>> +        }
>>         else
>>           {
>>             current_file = current_file->included_by;
>> @@ -737,7 +746,7 @@ dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,
>>                         current_file, lh, section,
>>                         section_is_gnu, is_dwz, offset_size,
>>                         str_section, str_offsets_section,
>> -                      str_offsets_base, include_hash);
>> +                      str_offsets_base, include_hash, cu);
>>           htab_remove_elt (include_hash, (void *) new_mac_ptr);
>>             }
>> @@ -781,7 +790,7 @@ dwarf_decode_macros (dwarf2_per_objfile *per_objfile,
>>                unsigned int offset, struct dwarf2_section_info *str_section,
>>                struct dwarf2_section_info *str_offsets_section,
>>                gdb::optional<ULONGEST> str_offsets_base,
>> -             int section_is_gnu)
>> +             int section_is_gnu, struct dwarf2_cu *cu)
>>   {
>>     bfd *abfd;
>>     const gdb_byte *mac_ptr, *mac_end;
>> @@ -942,5 +951,5 @@ dwarf_decode_macros (dwarf2_per_objfile *per_objfile,
>>     dwarf_decode_macro_bytes (per_objfile, builder, abfd, mac_ptr, mac_end,
>>                   current_file, lh, section, section_is_gnu, 0,
>>                   offset_size, str_section, str_offsets_section,
>> -                str_offsets_base, include_hash.get ());
>> +                str_offsets_base, include_hash.get (), cu);
>>   }
>> diff --git a/gdb/dwarf2/macro.h b/gdb/dwarf2/macro.h
>> index e8c33a34a8d..02753ef377a 100644
>> --- a/gdb/dwarf2/macro.h
>> +++ b/gdb/dwarf2/macro.h
>> @@ -31,6 +31,6 @@ extern void dwarf_decode_macros (dwarf2_per_objfile *per_objfile,
>>                    dwarf2_section_info *str_section,
>>                    dwarf2_section_info *str_offsets_section,
>>                    gdb::optional<ULONGEST> str_offsets_base,
>> -                 int section_is_gnu);
>> +                 int section_is_gnu, struct dwarf2_cu *cu);
>>   #endif /* GDB_DWARF2_MACRO_H */
>> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
>> index 698720276a9..1fc5d5d3dd8 100644
>> --- a/gdb/dwarf2/read.c
>> +++ b/gdb/dwarf2/read.c
>> @@ -9454,6 +9454,15 @@ producer_is_gcc_lt_4_3 (struct dwarf2_cu *cu)
>>     return cu->producer_is_gcc_lt_4_3;
>>   }
>> +bool
>> +producer_is_clang_le_14 (struct dwarf2_cu *cu)
>> +{
>> +  if (!cu->checked_producer)
>> +    check_producer (cu);
>> +
>> +  return cu->producer_is_clang_le_14;
>> +}
>> +
>>   static file_and_directory &
>>   find_file_and_directory (struct die_info *die, struct dwarf2_cu *cu)
>>   {
>> @@ -13295,6 +13304,10 @@ check_producer (struct dwarf2_cu *cu)
>>       }
>>     else if (startswith (cu->producer, "CodeWarrior S12/L-ISA"))
>>       cu->producer_is_codewarrior = true;
>> +  else if (producer_is_clang (cu->producer, &major, &minor))
>> +    {
>> +      cu->producer_is_clang_le_14 = major <= 14;
>> +    }
>>     else
>>       {
>>         /* For other non-GCC compilers, expect their behavior is DWARF version
>> @@ -23218,7 +23231,7 @@ dwarf_decode_macros (struct dwarf2_cu *cu, unsigned int offset,
>>     dwarf_decode_macros (per_objfile, builder, section, lh,
>>                  offset_size, offset, str_section, str_offsets_section,
>> -               str_offsets_base, section_is_gnu);
>> +               str_offsets_base, section_is_gnu, cu);
>>   }
>>   /* Return the .debug_loc section to use for CU.
>> diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
>> index be3b9d19601..23f040a6ca8 100644
>> --- a/gdb/dwarf2/read.h
>> +++ b/gdb/dwarf2/read.h
>> @@ -677,4 +677,6 @@ extern void dwarf2_get_section_info (struct objfile *,
>>                        asection **, const gdb_byte **,
>>                        bfd_size_type *);
>> +extern bool producer_is_clang_le_14(struct dwarf2_cu *cu);
>> +
>>   #endif /* DWARF2READ_H */
>> diff --git a/gdb/producer.c b/gdb/producer.c
>> index ef1dd93afbc..a9fb7dd8245 100644
>> --- a/gdb/producer.c
>> +++ b/gdb/producer.c
>> @@ -127,6 +127,32 @@ producer_is_llvm (const char *producer)
>>                    || startswith (producer, " F90 Flang ")));
>>   }
>> +/* see producer.h */
>> +
>> +bool
>> +producer_is_clang (const char* producer, int *major, int* minor)
>> +{
>> +  const char *cs;
>> +
>> +  if (producer != nullptr && startswith (producer, "clang "))
>> +    {
>> +      int maj, min;
>> +      if (major == nullptr)
>> +    major = &maj;
>> +      if (minor == nullptr)
>> +    minor = &min;
>> +
>> +      /* The full producer sting will look something like
>> +     "clang version XX.X.X ..."
>> +     So we can safely ignore all characters before the first digit.  */
>> +      cs = producer + strlen("clang version ");
>> +
>> +      if (sscanf (cs, "%d.%d", major, minor) == 2)
>> +      return true;
>> +    }
>> +  return false;
>> +}
>> +
>>   #if defined GDB_SELF_TEST
>>   namespace selftests {
>>   namespace producer {
>> diff --git a/gdb/producer.h b/gdb/producer.h
>> index f7c19368bc6..b75cfae6569 100644
>> --- a/gdb/producer.h
>> +++ b/gdb/producer.h
>> @@ -41,4 +41,8 @@ extern bool producer_is_icc (const char *producer, int *major, int *minor);
>>      false otherwise.*/
>>   extern bool producer_is_llvm (const char *producer);
>> +/* Returns true if the given PRODUCER string is clang, false otherwise.
>> +   Sets MAJOR and MINOR accordingly, if not NULL.  */
>> +extern bool producer_is_clang (const char *producer, int *major, int *minor);
>> +
>>   #endif

Re: [PATCH] gdb: Fix issue with Clang CLI macros

[Simon Marchi]

On 2022-04-20 13:41, Bruno Larsen via Gdb-patches wrote:
> Clang up to the current version adds macros that were defined in the
> command line or by "other means", according to the Dwarf specification,
> after the last DW_MACRO_end_file, instead of before the first
> DW_MACRO_start_file, as the specification dictates. This has been
> submitted as a bug to Clang developers, but seeing as there is no
> expected date for it to be fixed, a workaround was added for all current
> versions of Clang.
> 
> The workaround detects when the main file would be closed and if the
> producer is clang, and turns that operation into a noop, so we keep a
> reference to the current_file as those macros are read.
> 
> This patch fixes PR macros/29034, and can be tested by running
> gdb.base/macscp.exp using clang, the test printing FROM_COMMANDLINE
> should be fixed.

Hi Bruno,

Can we also add a DWARF-assembly-based test case, so that this is tested
regardless of CC_FOR_TARGET?

My series here adds some code to generate DWARF5 .debug_macro sections:

  https://sourceware.org/pipermail/gdb-patches/2022-April/188478.html

> ---
>  gdb/dwarf2/cu.h    |  1 +
>  gdb/dwarf2/macro.c | 17 +++++++++++++----
>  gdb/dwarf2/macro.h |  2 +-
>  gdb/dwarf2/read.c  | 15 ++++++++++++++-
>  gdb/dwarf2/read.h  |  2 ++
>  gdb/producer.c     | 26 ++++++++++++++++++++++++++
>  gdb/producer.h     |  4 ++++
>  7 files changed, 61 insertions(+), 6 deletions(-)
> 
> diff --git a/gdb/dwarf2/cu.h b/gdb/dwarf2/cu.h
> index 6b72ec234bf..3b7f92be406 100644
> --- a/gdb/dwarf2/cu.h
> +++ b/gdb/dwarf2/cu.h
> @@ -257,6 +257,7 @@ struct dwarf2_cu
>    bool producer_is_icc : 1;
>    bool producer_is_icc_lt_14 : 1;
>    bool producer_is_codewarrior : 1;
> +  bool producer_is_clang_le_14 : 1;
>  
>    /* When true, the file that we're processing is known to have
>       debugging info for C++ namespaces.  GCC 3.3.x did not produce
> diff --git a/gdb/dwarf2/macro.c b/gdb/dwarf2/macro.c
> index 99c3653a2c3..305af1a7f47 100644
> --- a/gdb/dwarf2/macro.c
> +++ b/gdb/dwarf2/macro.c
> @@ -431,7 +431,7 @@ dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,
>  			  struct dwarf2_section_info *str_section,
>  			  struct dwarf2_section_info *str_offsets_section,
>  			  gdb::optional<ULONGEST> str_offsets_base,
> -			  htab_t include_hash)
> +			  htab_t include_hash, struct dwarf2_cu *cu)

I think having both a dwarf2_per_objfile parameter and a dwarf2_cu parameter is
redundant, as you can get the per_objfile from the cu.  So perhaps just replace
the existing per_objfile parameter with the cu one (at the same position).

>  {
>    struct objfile *objfile = per_objfile->objfile;
>    enum dwarf_macro_record_type macinfo_type;
> @@ -658,6 +658,15 @@ dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,
>  	  if (! current_file)
>  	    complaint (_("macro debug info has an unmatched "
>  			 "`close_file' directive"));
> +	  else if (current_file->included_by == nullptr
> +		   && producer_is_clang_le_14 (cu))

Given that we don't really expect this to be fixed soon (nobody has expressed the
willingness to fix it), shouldn't we check for "producer_is_clang", without a
version check?  Otherwise this will need to be updated whenever a new Clang version
is released, if the bug hasn't been fixed.

> +	    {
> +	      /* Clang, until the current version, misplaces macro definitions,

It's not clear what "current version" means, as the definition of that changes with
time.  I would say "Clang (at the time of writing) misplaces...".  I would even put
a link to the Clang bug, so that people can easily go check if that is still the case
or if that has been fixed.

You should say "command-line macro definitions (e.g. -DFOO=1)", because it's not
all macro definitions.



> +		 putting them after the last DW_MACRO_end_file instead of
> +		 before the first DW_MACRO_start_file.  This condition should be
> +		 removed once we can reasonably expect most clang users to have
> +		 updated to a fixed version.  */
> +	    }
>  	  else
>  	    {
>  	      current_file = current_file->included_by;
> @@ -737,7 +746,7 @@ dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,
>  					  current_file, lh, section,
>  					  section_is_gnu, is_dwz, offset_size,
>  					  str_section, str_offsets_section,
> -					  str_offsets_base, include_hash);
> +					  str_offsets_base, include_hash, cu);
>  
>  		htab_remove_elt (include_hash, (void *) new_mac_ptr);
>  	      }
> @@ -781,7 +790,7 @@ dwarf_decode_macros (dwarf2_per_objfile *per_objfile,
>  		     unsigned int offset, struct dwarf2_section_info *str_section,
>  		     struct dwarf2_section_info *str_offsets_section,
>  		     gdb::optional<ULONGEST> str_offsets_base,
> -		     int section_is_gnu)
> +		     int section_is_gnu, struct dwarf2_cu *cu)
>  {
>    bfd *abfd;
>    const gdb_byte *mac_ptr, *mac_end;
> @@ -942,5 +951,5 @@ dwarf_decode_macros (dwarf2_per_objfile *per_objfile,
>    dwarf_decode_macro_bytes (per_objfile, builder, abfd, mac_ptr, mac_end,
>  			    current_file, lh, section, section_is_gnu, 0,
>  			    offset_size, str_section, str_offsets_section,
> -			    str_offsets_base, include_hash.get ());
> +			    str_offsets_base, include_hash.get (), cu);
>  }
> diff --git a/gdb/dwarf2/macro.h b/gdb/dwarf2/macro.h
> index e8c33a34a8d..02753ef377a 100644
> --- a/gdb/dwarf2/macro.h
> +++ b/gdb/dwarf2/macro.h
> @@ -31,6 +31,6 @@ extern void dwarf_decode_macros (dwarf2_per_objfile *per_objfile,
>  				 dwarf2_section_info *str_section,
>  				 dwarf2_section_info *str_offsets_section,
>  				 gdb::optional<ULONGEST> str_offsets_base,
> -				 int section_is_gnu);
> +				 int section_is_gnu, struct dwarf2_cu *cu);
>  
>  #endif /* GDB_DWARF2_MACRO_H */
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 698720276a9..1fc5d5d3dd8 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -9454,6 +9454,15 @@ producer_is_gcc_lt_4_3 (struct dwarf2_cu *cu)
>    return cu->producer_is_gcc_lt_4_3;
>  }
>  
> +bool
> +producer_is_clang_le_14 (struct dwarf2_cu *cu)
> +{
> +  if (!cu->checked_producer)
> +    check_producer (cu);
> +  
> +  return cu->producer_is_clang_le_14;
> +}
> +
>  static file_and_directory &
>  find_file_and_directory (struct die_info *die, struct dwarf2_cu *cu)
>  {
> @@ -13295,6 +13304,10 @@ check_producer (struct dwarf2_cu *cu)
>      }
>    else if (startswith (cu->producer, "CodeWarrior S12/L-ISA"))
>      cu->producer_is_codewarrior = true;
> +  else if (producer_is_clang (cu->producer, &major, &minor))
> +    {
> +      cu->producer_is_clang_le_14 = major <= 14;
> +    }

Remove braces.

>    else
>      {
>        /* For other non-GCC compilers, expect their behavior is DWARF version
> @@ -23218,7 +23231,7 @@ dwarf_decode_macros (struct dwarf2_cu *cu, unsigned int offset,
>  
>    dwarf_decode_macros (per_objfile, builder, section, lh,
>  		       offset_size, offset, str_section, str_offsets_section,
> -		       str_offsets_base, section_is_gnu);
> +		       str_offsets_base, section_is_gnu, cu);
>  }
>  
>  /* Return the .debug_loc section to use for CU.
> diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
> index be3b9d19601..23f040a6ca8 100644
> --- a/gdb/dwarf2/read.h
> +++ b/gdb/dwarf2/read.h
> @@ -677,4 +677,6 @@ extern void dwarf2_get_section_info (struct objfile *,
>  				     asection **, const gdb_byte **,
>  				     bfd_size_type *);
>  
> +extern bool producer_is_clang_le_14(struct dwarf2_cu *cu);
> +
>  #endif /* DWARF2READ_H */
> diff --git a/gdb/producer.c b/gdb/producer.c
> index ef1dd93afbc..a9fb7dd8245 100644
> --- a/gdb/producer.c
> +++ b/gdb/producer.c
> @@ -127,6 +127,32 @@ producer_is_llvm (const char *producer)
>  				 || startswith (producer, " F90 Flang ")));

Just noting that users of these functions will need to be careful, as there's
an overlap between producer_is_llvm and producer_is_clang.

>  }
>  
> +/* see producer.h */

Capital S, period + two spaces at the end.

> +
> +bool
> +producer_is_clang (const char* producer, int *major, int* minor)

Space before * (twice).

> +{
> +  const char *cs;

Declare in the inner scope (at the point of use).

> +
> +  if (producer != nullptr && startswith (producer, "clang "))
> +    {
> +      int maj, min;
> +      if (major == nullptr)
> +	major = &maj;
> +      if (minor == nullptr)
> +	minor = &min;
> +
> +      /* The full producer sting will look something like

sting -> string

> +	 "clang version XX.X.X ..."
> +	 So we can safely ignore all characters before the first digit.  */
> +      cs = producer + strlen("clang version ");

Space after strlen.

> +
> +      if (sscanf (cs, "%d.%d", major, minor) == 2)
> +	  return true;

Remove one indentation level here.

> +    }
> +  return false;
> +}

Can you add some tests for this in producer_parsing_tests?

Simon

Re: [PATCH] gdb: Fix issue with Clang CLI macros

[Bruno Larsen]

On 5/11/22 09:59, Simon Marchi wrote:
> 
> 
> On 2022-04-20 13:41, Bruno Larsen via Gdb-patches wrote:
>> Clang up to the current version adds macros that were defined in the
>> command line or by "other means", according to the Dwarf specification,
>> after the last DW_MACRO_end_file, instead of before the first
>> DW_MACRO_start_file, as the specification dictates. This has been
>> submitted as a bug to Clang developers, but seeing as there is no
>> expected date for it to be fixed, a workaround was added for all current
>> versions of Clang.
>>
>> The workaround detects when the main file would be closed and if the
>> producer is clang, and turns that operation into a noop, so we keep a
>> reference to the current_file as those macros are read.
>>
>> This patch fixes PR macros/29034, and can be tested by running
>> gdb.base/macscp.exp using clang, the test printing FROM_COMMANDLINE
>> should be fixed.
> 
> Hi Bruno,

Hi Simon!

Thanks for the review, I've fixed all minor comments locally and will send a v2 shortly.

> 
> Can we also add a DWARF-assembly-based test case, so that this is tested
> regardless of CC_FOR_TARGET?

  I can do that, but since I have a producer check and I want to specifically fix this clang bug, the test would want to see nothing happening on regular testing, and the macro showing up for clang. Is this what you had in mind?


> 
> My series here adds some code to generate DWARF5 .debug_macro sections:
> 
>    https://sourceware.org/pipermail/gdb-patches/2022-April/188478.html
> 
>> ---
>>   gdb/dwarf2/cu.h    |  1 +
>>   gdb/dwarf2/macro.c | 17 +++++++++++++----
>>   gdb/dwarf2/macro.h |  2 +-
>>   gdb/dwarf2/read.c  | 15 ++++++++++++++-
>>   gdb/dwarf2/read.h  |  2 ++
>>   gdb/producer.c     | 26 ++++++++++++++++++++++++++
>>   gdb/producer.h     |  4 ++++
>>   7 files changed, 61 insertions(+), 6 deletions(-)
>>
>> diff --git a/gdb/dwarf2/cu.h b/gdb/dwarf2/cu.h
>> index 6b72ec234bf..3b7f92be406 100644
>> --- a/gdb/dwarf2/cu.h
>> +++ b/gdb/dwarf2/cu.h
>> @@ -257,6 +257,7 @@ struct dwarf2_cu
>>     bool producer_is_icc : 1;
>>     bool producer_is_icc_lt_14 : 1;
>>     bool producer_is_codewarrior : 1;
>> +  bool producer_is_clang_le_14 : 1;
>>   
>>     /* When true, the file that we're processing is known to have
>>        debugging info for C++ namespaces.  GCC 3.3.x did not produce
>> diff --git a/gdb/dwarf2/macro.c b/gdb/dwarf2/macro.c
>> index 99c3653a2c3..305af1a7f47 100644
>> --- a/gdb/dwarf2/macro.c
>> +++ b/gdb/dwarf2/macro.c
>> @@ -431,7 +431,7 @@ dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,
>>   			  struct dwarf2_section_info *str_section,
>>   			  struct dwarf2_section_info *str_offsets_section,
>>   			  gdb::optional<ULONGEST> str_offsets_base,
>> -			  htab_t include_hash)
>> +			  htab_t include_hash, struct dwarf2_cu *cu)
> 
> I think having both a dwarf2_per_objfile parameter and a dwarf2_cu parameter is
> redundant, as you can get the per_objfile from the cu.  So perhaps just replace
> the existing per_objfile parameter with the cu one (at the same position).
> 
>>   {
>>     struct objfile *objfile = per_objfile->objfile;
>>     enum dwarf_macro_record_type macinfo_type;
>> @@ -658,6 +658,15 @@ dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,
>>   	  if (! current_file)
>>   	    complaint (_("macro debug info has an unmatched "
>>   			 "`close_file' directive"));
>> +	  else if (current_file->included_by == nullptr
>> +		   && producer_is_clang_le_14 (cu))
> 
> Given that we don't really expect this to be fixed soon (nobody has expressed the
> willingness to fix it), shouldn't we check for "producer_is_clang", without a
> version check?  Otherwise this will need to be updated whenever a new Clang version
> is released, if the bug hasn't been fixed.

Sure! I just didn't want us to forget this was supposed to be a quick bug fix in a few months, so I went with the overzealous route.

I do still think this should be updated to producer_is_clang_le_XX once they fix it, to guarantee that they don't regress.

> 
>> +	    {
>> +	      /* Clang, until the current version, misplaces macro definitions,
> 
> It's not clear what "current version" means, as the definition of that changes with
> time.  I would say "Clang (at the time of writing) misplaces...".  I would even put
> a link to the Clang bug, so that people can easily go check if that is still the case
> or if that has been fixed.
> 
> You should say "command-line macro definitions (e.g. -DFOO=1)", because it's not
> all macro definitions.> 
> 
> 
>> +		 putting them after the last DW_MACRO_end_file instead of
>> +		 before the first DW_MACRO_start_file.  This condition should be
>> +		 removed once we can reasonably expect most clang users to have
>> +		 updated to a fixed version.  */
>> +	    }
>>   	  else
>>   	    {
>>   	      current_file = current_file->included_by;
>> @@ -737,7 +746,7 @@ dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,
>>   					  current_file, lh, section,
>>   					  section_is_gnu, is_dwz, offset_size,
>>   					  str_section, str_offsets_section,
>> -					  str_offsets_base, include_hash);
>> +					  str_offsets_base, include_hash, cu);
>>   
>>   		htab_remove_elt (include_hash, (void *) new_mac_ptr);
>>   	      }
>> @@ -781,7 +790,7 @@ dwarf_decode_macros (dwarf2_per_objfile *per_objfile,
>>   		     unsigned int offset, struct dwarf2_section_info *str_section,
>>   		     struct dwarf2_section_info *str_offsets_section,
>>   		     gdb::optional<ULONGEST> str_offsets_base,
>> -		     int section_is_gnu)
>> +		     int section_is_gnu, struct dwarf2_cu *cu)
>>   {
>>     bfd *abfd;
>>     const gdb_byte *mac_ptr, *mac_end;
>> @@ -942,5 +951,5 @@ dwarf_decode_macros (dwarf2_per_objfile *per_objfile,
>>     dwarf_decode_macro_bytes (per_objfile, builder, abfd, mac_ptr, mac_end,
>>   			    current_file, lh, section, section_is_gnu, 0,
>>   			    offset_size, str_section, str_offsets_section,
>> -			    str_offsets_base, include_hash.get ());
>> +			    str_offsets_base, include_hash.get (), cu);
>>   }
>> diff --git a/gdb/dwarf2/macro.h b/gdb/dwarf2/macro.h
>> index e8c33a34a8d..02753ef377a 100644
>> --- a/gdb/dwarf2/macro.h
>> +++ b/gdb/dwarf2/macro.h
>> @@ -31,6 +31,6 @@ extern void dwarf_decode_macros (dwarf2_per_objfile *per_objfile,
>>   				 dwarf2_section_info *str_section,
>>   				 dwarf2_section_info *str_offsets_section,
>>   				 gdb::optional<ULONGEST> str_offsets_base,
>> -				 int section_is_gnu);
>> +				 int section_is_gnu, struct dwarf2_cu *cu);
>>   
>>   #endif /* GDB_DWARF2_MACRO_H */
>> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
>> index 698720276a9..1fc5d5d3dd8 100644
>> --- a/gdb/dwarf2/read.c
>> +++ b/gdb/dwarf2/read.c
>> @@ -9454,6 +9454,15 @@ producer_is_gcc_lt_4_3 (struct dwarf2_cu *cu)
>>     return cu->producer_is_gcc_lt_4_3;
>>   }
>>   
>> +bool
>> +producer_is_clang_le_14 (struct dwarf2_cu *cu)
>> +{
>> +  if (!cu->checked_producer)
>> +    check_producer (cu);
>> +
>> +  return cu->producer_is_clang_le_14;
>> +}
>> +
>>   static file_and_directory &
>>   find_file_and_directory (struct die_info *die, struct dwarf2_cu *cu)
>>   {
>> @@ -13295,6 +13304,10 @@ check_producer (struct dwarf2_cu *cu)
>>       }
>>     else if (startswith (cu->producer, "CodeWarrior S12/L-ISA"))
>>       cu->producer_is_codewarrior = true;
>> +  else if (producer_is_clang (cu->producer, &major, &minor))
>> +    {
>> +      cu->producer_is_clang_le_14 = major <= 14;
>> +    }
> 
> Remove braces.
> 
>>     else
>>       {
>>         /* For other non-GCC compilers, expect their behavior is DWARF version
>> @@ -23218,7 +23231,7 @@ dwarf_decode_macros (struct dwarf2_cu *cu, unsigned int offset,
>>   
>>     dwarf_decode_macros (per_objfile, builder, section, lh,
>>   		       offset_size, offset, str_section, str_offsets_section,
>> -		       str_offsets_base, section_is_gnu);
>> +		       str_offsets_base, section_is_gnu, cu);
>>   }
>>   
>>   /* Return the .debug_loc section to use for CU.
>> diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
>> index be3b9d19601..23f040a6ca8 100644
>> --- a/gdb/dwarf2/read.h
>> +++ b/gdb/dwarf2/read.h
>> @@ -677,4 +677,6 @@ extern void dwarf2_get_section_info (struct objfile *,
>>   				     asection **, const gdb_byte **,
>>   				     bfd_size_type *);
>>   
>> +extern bool producer_is_clang_le_14(struct dwarf2_cu *cu);
>> +
>>   #endif /* DWARF2READ_H */
>> diff --git a/gdb/producer.c b/gdb/producer.c
>> index ef1dd93afbc..a9fb7dd8245 100644
>> --- a/gdb/producer.c
>> +++ b/gdb/producer.c
>> @@ -127,6 +127,32 @@ producer_is_llvm (const char *producer)
>>   				 || startswith (producer, " F90 Flang ")));
> 
> Just noting that users of these functions will need to be careful, as there's
> an overlap between producer_is_llvm and producer_is_clang.
> 
>>   }
>>   
>> +/* see producer.h */
> 
> Capital S, period + two spaces at the end.
> 
>> +
>> +bool
>> +producer_is_clang (const char* producer, int *major, int* minor)
> 
> Space before * (twice).
> 
>> +{
>> +  const char *cs;
> 
> Declare in the inner scope (at the point of use).
> 
>> +
>> +  if (producer != nullptr && startswith (producer, "clang "))
>> +    {
>> +      int maj, min;
>> +      if (major == nullptr)
>> +	major = &maj;
>> +      if (minor == nullptr)
>> +	minor = &min;
>> +
>> +      /* The full producer sting will look something like
> 
> sting -> string
> 
>> +	 "clang version XX.X.X ..."
>> +	 So we can safely ignore all characters before the first digit.  */
>> +      cs = producer + strlen("clang version ");
> 
> Space after strlen.
> 
>> +
>> +      if (sscanf (cs, "%d.%d", major, minor) == 2)
>> +	  return true;
> 
> Remove one indentation level here.
> 
>> +    }
>> +  return false;
>> +}
> 
> Can you add some tests for this in producer_parsing_tests?> 
> Simon
> 

Cheers!
Bruno Larsen

Re: [PATCH] gdb: Fix issue with Clang CLI macros

[Simon Marchi]

On 2022-05-11 10:53, Bruno Larsen via Gdb-patches wrote:
> 
> On 5/11/22 09:59, Simon Marchi wrote:
>>
>>
>> On 2022-04-20 13:41, Bruno Larsen via Gdb-patches wrote:
>>> Clang up to the current version adds macros that were defined in the
>>> command line or by "other means", according to the Dwarf specification,
>>> after the last DW_MACRO_end_file, instead of before the first
>>> DW_MACRO_start_file, as the specification dictates. This has been
>>> submitted as a bug to Clang developers, but seeing as there is no
>>> expected date for it to be fixed, a workaround was added for all current
>>> versions of Clang.
>>>
>>> The workaround detects when the main file would be closed and if the
>>> producer is clang, and turns that operation into a noop, so we keep a
>>> reference to the current_file as those macros are read.
>>>
>>> This patch fixes PR macros/29034, and can be tested by running
>>> gdb.base/macscp.exp using clang, the test printing FROM_COMMANDLINE
>>> should be fixed.
>>
>> Hi Bruno,
> 
> Hi Simon!
> 
> Thanks for the review, I've fixed all minor comments locally and will send a v2 shortly.
> 
>>
>> Can we also add a DWARF-assembly-based test case, so that this is tested
>> regardless of CC_FOR_TARGET?
> 
>  I can do that, but since I have a producer check and I want to specifically fix this clang bug, the test would want to see nothing happening on regular testing, and the macro showing up for clang. Is this what you had in mind?

The test you produce could include the "clang" producer string.

> 
> 
>>
>> My series here adds some code to generate DWARF5 .debug_macro sections:
>>
>>    https://sourceware.org/pipermail/gdb-patches/2022-April/188478.html
>>
>>> ---
>>>   gdb/dwarf2/cu.h    |  1 +
>>>   gdb/dwarf2/macro.c | 17 +++++++++++++----
>>>   gdb/dwarf2/macro.h |  2 +-
>>>   gdb/dwarf2/read.c  | 15 ++++++++++++++-
>>>   gdb/dwarf2/read.h  |  2 ++
>>>   gdb/producer.c     | 26 ++++++++++++++++++++++++++
>>>   gdb/producer.h     |  4 ++++
>>>   7 files changed, 61 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/gdb/dwarf2/cu.h b/gdb/dwarf2/cu.h
>>> index 6b72ec234bf..3b7f92be406 100644
>>> --- a/gdb/dwarf2/cu.h
>>> +++ b/gdb/dwarf2/cu.h
>>> @@ -257,6 +257,7 @@ struct dwarf2_cu
>>>     bool producer_is_icc : 1;
>>>     bool producer_is_icc_lt_14 : 1;
>>>     bool producer_is_codewarrior : 1;
>>> +  bool producer_is_clang_le_14 : 1;
>>>       /* When true, the file that we're processing is known to have
>>>        debugging info for C++ namespaces.  GCC 3.3.x did not produce
>>> diff --git a/gdb/dwarf2/macro.c b/gdb/dwarf2/macro.c
>>> index 99c3653a2c3..305af1a7f47 100644
>>> --- a/gdb/dwarf2/macro.c
>>> +++ b/gdb/dwarf2/macro.c
>>> @@ -431,7 +431,7 @@ dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,
>>>                 struct dwarf2_section_info *str_section,
>>>                 struct dwarf2_section_info *str_offsets_section,
>>>                 gdb::optional<ULONGEST> str_offsets_base,
>>> -              htab_t include_hash)
>>> +              htab_t include_hash, struct dwarf2_cu *cu)
>>
>> I think having both a dwarf2_per_objfile parameter and a dwarf2_cu parameter is
>> redundant, as you can get the per_objfile from the cu.  So perhaps just replace
>> the existing per_objfile parameter with the cu one (at the same position).
>>
>>>   {
>>>     struct objfile *objfile = per_objfile->objfile;
>>>     enum dwarf_macro_record_type macinfo_type;
>>> @@ -658,6 +658,15 @@ dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,
>>>         if (! current_file)
>>>           complaint (_("macro debug info has an unmatched "
>>>                "`close_file' directive"));
>>> +      else if (current_file->included_by == nullptr
>>> +           && producer_is_clang_le_14 (cu))
>>
>> Given that we don't really expect this to be fixed soon (nobody has expressed the
>> willingness to fix it), shouldn't we check for "producer_is_clang", without a
>> version check?  Otherwise this will need to be updated whenever a new Clang version
>> is released, if the bug hasn't been fixed.
> 
> Sure! I just didn't want us to forget this was supposed to be a quick bug fix in a few months, so I went with the overzealous route.
> 
> I do still think this should be updated to producer_is_clang_le_XX once they fix it, to guarantee that they don't regress.

Agreed, but for now we don't know whether / when they'll fix it.

Simon

Re: [PATCH] gdb: Fix issue with Clang CLI macros

[Bruno Larsen]

On 5/11/22 15:48, Simon Marchi wrote:
> 
> 
> On 2022-05-11 10:53, Bruno Larsen via Gdb-patches wrote:
>>
>> On 5/11/22 09:59, Simon Marchi wrote:
>>>
>>>
>>> On 2022-04-20 13:41, Bruno Larsen via Gdb-patches wrote:
>>>> Clang up to the current version adds macros that were defined in the
>>>> command line or by "other means", according to the Dwarf specification,
>>>> after the last DW_MACRO_end_file, instead of before the first
>>>> DW_MACRO_start_file, as the specification dictates. This has been
>>>> submitted as a bug to Clang developers, but seeing as there is no
>>>> expected date for it to be fixed, a workaround was added for all current
>>>> versions of Clang.
>>>>
>>>> The workaround detects when the main file would be closed and if the
>>>> producer is clang, and turns that operation into a noop, so we keep a
>>>> reference to the current_file as those macros are read.
>>>>
>>>> This patch fixes PR macros/29034, and can be tested by running
>>>> gdb.base/macscp.exp using clang, the test printing FROM_COMMANDLINE
>>>> should be fixed.
>>>
>>> Hi Bruno,
>>
>> Hi Simon!
>>
>> Thanks for the review, I've fixed all minor comments locally and will send a v2 shortly.
>>
>>>
>>> Can we also add a DWARF-assembly-based test case, so that this is tested
>>> regardless of CC_FOR_TARGET?
>>
>>   I can do that, but since I have a producer check and I want to specifically fix this clang bug, the test would want to see nothing happening on regular testing, and the macro showing up for clang. Is this what you had in mind?
> 
> The test you produce could include the "clang" producer string.

Right, yes, sorry, didn't think we could do that.

> 
>>
>>
>>>
>>> My series here adds some code to generate DWARF5 .debug_macro sections:
>>>
>>>     https://sourceware.org/pipermail/gdb-patches/2022-April/188478.html
>>>

Has this series been merged already? I've just pulled master and I couldn't find the code for macros in testsuite/lib/dwarf.exp


Cheers!
Bruno Larsen

Re: [PATCH] gdb: Fix issue with Clang CLI macros

[Simon Marchi]

On 2022-05-11 16:03, Bruno Larsen via Gdb-patches wrote:
> 
> On 5/11/22 15:48, Simon Marchi wrote:
>>
>>
>> On 2022-05-11 10:53, Bruno Larsen via Gdb-patches wrote:
>>>
>>> On 5/11/22 09:59, Simon Marchi wrote:
>>>>
>>>>
>>>> On 2022-04-20 13:41, Bruno Larsen via Gdb-patches wrote:
>>>>> Clang up to the current version adds macros that were defined in the
>>>>> command line or by "other means", according to the Dwarf specification,
>>>>> after the last DW_MACRO_end_file, instead of before the first
>>>>> DW_MACRO_start_file, as the specification dictates. This has been
>>>>> submitted as a bug to Clang developers, but seeing as there is no
>>>>> expected date for it to be fixed, a workaround was added for all current
>>>>> versions of Clang.
>>>>>
>>>>> The workaround detects when the main file would be closed and if the
>>>>> producer is clang, and turns that operation into a noop, so we keep a
>>>>> reference to the current_file as those macros are read.
>>>>>
>>>>> This patch fixes PR macros/29034, and can be tested by running
>>>>> gdb.base/macscp.exp using clang, the test printing FROM_COMMANDLINE
>>>>> should be fixed.
>>>>
>>>> Hi Bruno,
>>>
>>> Hi Simon!
>>>
>>> Thanks for the review, I've fixed all minor comments locally and will send a v2 shortly.
>>>
>>>>
>>>> Can we also add a DWARF-assembly-based test case, so that this is tested
>>>> regardless of CC_FOR_TARGET?
>>>
>>>   I can do that, but since I have a producer check and I want to specifically fix this clang bug, the test would want to see nothing happening on regular testing, and the macro showing up for clang. Is this what you had in mind?
>>
>> The test you produce could include the "clang" producer string.
> 
> Right, yes, sorry, didn't think we could do that.
> 
>>
>>>
>>>
>>>>
>>>> My series here adds some code to generate DWARF5 .debug_macro sections:
>>>>
>>>>     https://sourceware.org/pipermail/gdb-patches/2022-April/188478.html
>>>>
> 
> Has this series been merged already? I've just pulled master and I couldn't find the code for macros in testsuite/lib/dwarf.exp

No, I'm waiting for review :).

Simon