gprofng SIGSEGV when processing unusual dwarf

gprofng SIGSEGV when processing unusual dwarf

[Gilles DUBOSCQ]

Hi all,

When running gprofng display text on some executables that contain unusual dwarf data I am getting a segfault.
In particular, the input dwarf has some compilation units (DW_TAG_compile_unit) that have neither DW_AT_comp_dir nor DW_AT_stmt_list.

The issue is that when that happens, DwrCU::stmt_list_offset remains 0, as a result, in Dwarf::archive_Dwarf, the `get_dwrLineReg` call will process whatever is at offset 0 in .debug_line.
Then while looking for source files, `DwrLineRegs::getPath` will potentially try to use include_directories->fetch (0) which is NULL (no comp_dir attribute).
This leads to a segfault in StringBuilder::append.

A quick local test show that the issue goes away with the change below.
The change in DwrLineRegs::getPath is not strictly necessary but makes the code more robust if some compilation unit has stmt_list but not comp_dir.

Does it make sense? How could we get a fix for this integrated?
If that helps, I can create a bugzilla issue.

Thanks,
 Gilles

diff --git a/gprofng/src/Dwarf.cc b/gprofng/src/Dwarf.cc
index 1b33ae8f243..b36fc424924 100644
--- a/gprofng/src/Dwarf.cc
+++ b/gprofng/src/Dwarf.cc
@@ -606,6 +606,8 @@ Dwarf::archive_Dwarf (LoadObject *lo)
 	{
 	  mod->hdrOffset = dwrCUs->size ();
 	  DwrLineRegs *lineReg = dwrCU->get_dwrLineReg ();
+          if (lineReg != NULL)
+            {
               dwrCU->srcFiles = new Vector<SourceFile *>(VecSize (lineReg->file_names));
               for (long i = 0, sz = VecSize (lineReg->file_names); i < sz; i++)
                 {
@@ -613,6 +615,7 @@ Dwarf::archive_Dwarf (LoadObject *lo)
                   SourceFile *sf = mod->findSource(fname, true);
                   dwrCU->srcFiles->append(sf);
                 }
+            }
 
 	  Dwarf_cnt ctx;
 	  ctx.module = mod;
@@ -986,9 +989,6 @@ DwrCU::append_Function (Dwarf_cnt *ctx)
       if (lineno > 0)
 	{
 	  func->setLineFirst (lineno);
-	  if (dwrLineReg == NULL)
-	    dwrLineReg = new DwrLineRegs (new DwrSec (dwarf->debug_lineSec,
-						   stmt_list_offset), comp_dir);
 	  int fileno = ((int) Dwarf_data (DW_AT_decl_file)) - 1;
 	  SourceFile *sf = ((fileno >= 0) && (fileno < VecSize (srcFiles))) ? srcFiles->get (fileno)
 		  : module->getMainSrc ();
diff --git a/gprofng/src/DwarfLib.cc b/gprofng/src/DwarfLib.cc
index 4f86a78d1c8..ad2f95be9fa 100644
--- a/gprofng/src/DwarfLib.cc
+++ b/gprofng/src/DwarfLib.cc
@@ -1557,9 +1557,12 @@ DwrLineRegs::getPath (int fn)
   if (*dir != '/')
     { // not absolute
       char *s = include_directories->fetch (0);
+      if (s != NULL && *s != 0)
+        {
           sb.append(s);
           sb.append('/');
         }
+    }
   sb.append (dir);
   sb.append ('/');
   sb.append (fnp->fname);
@@ -1590,7 +1593,7 @@ DwrCU::DwrCU (Dwarf *_dwarf)
   abbrevTable = NULL;
   dwrInlinedSubrs = NULL;
   srcFiles = NULL;
-  stmt_list_offset = 0;
+  stmt_list_offset = NO_STMT_LIST;
   dwrLineReg = NULL;
   isMemop = false;
   isGNU = false;
@@ -1857,7 +1860,7 @@ DwrCU::parse_cu_header (LoadObject *lo)
   char *name = Dwarf_string (DW_AT_name);
   if (name == NULL)
     name = NTXT ("UnnamedUnit");
-  stmt_list_offset = Dwarf_data (DW_AT_stmt_list);
+  read_ref_attr(DW_AT_stmt_list, reinterpret_cast<int64_t *>(&stmt_list_offset));
   comp_dir = dbe_strdup (Dwarf_string (DW_AT_comp_dir));
   char *dir_name = comp_dir ? StrChr (comp_dir, ':') : NULL;
   char *orig_name = Dwarf_string (DW_AT_SUN_original_name);
@@ -2073,6 +2076,8 @@ DwrCU::map_dwarf_lines (Module *mod)
 					Stabs::is_fortran (mod->lang_code));
 	}
     }
+  if (lineReg == NULL)
+      return;
   Vector<DwrLine *> *lines = lineReg->get_lines ();
 
   Include *includes = new Include;
@@ -2083,7 +2088,7 @@ DwrCU::map_dwarf_lines (Module *mod)
   for (long i = 0, sz = VecSize (lines); i < sz; i++)
     {
       DwrLine *dwrLine = lines->get (i);
-      char *filename = dwrLineReg->getPath (dwrLine->file);
+      char *filename = lineReg->getPath (dwrLine->file);
       if (filename == NULL)
 	continue;
       uint64_t pc = dwrLine->address;
@@ -2123,7 +2128,7 @@ DwrCU::map_dwarf_lines (Module *mod)
 DwrLineRegs *
 DwrCU::get_dwrLineReg ()
 {
-  if (dwrLineReg == NULL)
+  if (dwrLineReg == NULL && stmt_list_offset != NO_STMT_LIST)
     dwrLineReg = new DwrLineRegs (new DwrSec (dwarf->debug_lineSec,
 					      stmt_list_offset), comp_dir);
   return dwrLineReg;
diff --git a/gprofng/src/DwarfLib.h b/gprofng/src/DwarfLib.h
index d359c7583b1..591d7d548c9 100644
--- a/gprofng/src/DwarfLib.h
+++ b/gprofng/src/DwarfLib.h
@@ -254,6 +254,8 @@ public:
   Dwr_type *put_dwr_type (Dwr_Tag *dwrTag);
 };
 
+#define NO_STMT_LIST 0xffffffffffffffffULL
+
 class DwrCU
 {
 public:

Re: gprofng SIGSEGV when processing unusual dwarf

[Ruud van der Pas]

Hi Gilles,

Thank you very much for your posting.

> When running gprofng display text on some executables that contain unusual dwarf data I am getting a segfault.
> In particular, the input dwarf has some compilation units (DW_TAG_compile_unit) that have neither DW_AT_comp_dir nor DW_AT_stmt_list.

I'm sorry to hear this.

> The issue is that when that happens, DwrCU::stmt_list_offset remains 0, as a result, in Dwarf::archive_Dwarf, the `get_dwrLineReg` call will process whatever is at offset 0 in .debug_line.
> Then while looking for source files, `DwrLineRegs::getPath` will potentially try to use include_directories->fetch (0) which is NULL (no comp_dir attribute).
> This leads to a segfault in StringBuilder::append.

Thanks for the analysis of the root cause of the segfault!

> A quick local test show that the issue goes away with the change below.
> The change in DwrLineRegs::getPath is not strictly necessary but makes the code more robust if some compilation unit has stmt_list but not comp_dir.

That sounds really good. Thank you for working on a fix!

> Does it make sense? How could we get a fix for this integrated?

We can take your changes, do some more testing, and then submit a patch.

> If that helps, I can create a bugzilla issue.

Thanks.

Although even more work for you, tracking these things in bugzilla helps.

Others can then more easily check whether they're running into the same problem
and we can connect an upcoming patch with the bugzilla id.

But if this is too much work for you, we can also do this.

Kind regards, Ruud

Re: gprofng SIGSEGV when processing unusual dwarf

[Gilles DUBOSCQ]

Hi Ruud,
Thanks for looking at this, I have created #30093 in bugzilla.
 Gilles
PS: as a long-time user of the sun/solaris/oracle developer studio profiler, i'm very happy to see it continue its life as gprofng :)

________________________________________
From: Ruud van der Pas <ruud.vanderpas@oracle.com>
Sent: Monday, 6 February 2023 21:28
To: Gilles DUBOSCQ
Cc: binutils@sourceware.org
Subject: Re: gprofng SIGSEGV when processing unusual dwarf

Hi Gilles,

Thank you very much for your posting.

> When running gprofng display text on some executables that contain unusual dwarf data I am getting a segfault.
> In particular, the input dwarf has some compilation units (DW_TAG_compile_unit) that have neither DW_AT_comp_dir nor DW_AT_stmt_list.

I'm sorry to hear this.

> The issue is that when that happens, DwrCU::stmt_list_offset remains 0, as a result, in Dwarf::archive_Dwarf, the `get_dwrLineReg` call will process whatever is at offset 0 in .debug_line.
> Then while looking for source files, `DwrLineRegs::getPath` will potentially try to use include_directories->fetch (0) which is NULL (no comp_dir attribute).
> This leads to a segfault in StringBuilder::append.

Thanks for the analysis of the root cause of the segfault!

> A quick local test show that the issue goes away with the change below.
> The change in DwrLineRegs::getPath is not strictly necessary but makes the code more robust if some compilation unit has stmt_list but not comp_dir.

That sounds really good. Thank you for working on a fix!

> Does it make sense? How could we get a fix for this integrated?

We can take your changes, do some more testing, and then submit a patch.

> If that helps, I can create a bugzilla issue.

Thanks.

Although even more work for you, tracking these things in bugzilla helps.

Others can then more easily check whether they're running into the same problem
and we can connect an upcoming patch with the bugzilla id.

But if this is too much work for you, we can also do this.

Kind regards, Ruud

Re: gprofng SIGSEGV when processing unusual dwarf

[Gilles DUBOSCQ]

Hi Vladimir,

I saw you posted a patch for this. A few remarks:
* Regarding the NO_STMT_LIST, i'm not sure what it the correct/portable way of defining uint64_t constants, i used a ULL literal for my tests but maybe UINT64_MAX should be used instead? I'm not sure exactly if this is available in all the environments where gprofng is built.
* Regarding the change in DwrCU::parse_cu_header, i think Dwarf_ref will also return 0 when the attribute is not found, what i was trying to achieve in my tests was for stmt_list_offset to be kept at its default NO_STMT_LIST value if the attribute is absent.

Best regards,
 Gilles

Re: gprofng SIGSEGV when processing unusual dwarf

[Vladimir Mezentsev]

Hi Gilles,



On 2/7/23 01:39, Gilles DUBOSCQ wrote:
> Hi Vladimir,
>
> I saw you posted a patch for this. A few remarks:
> * Regarding the NO_STMT_LIST, i'm not sure what it the correct/portable way of defining uint64_t constants, i used a ULL literal for my tests but maybe UINT64_MAX should be used instead? I'm not sure exactly if this is available in all the environments where gprofng is built.

  I see that we use  ((uint64_t)(-1)).
I made thee similar fix for  NO_STMT_LIST:

#define NO_STMT_LIST ((uint64_t) -1)


> * Regarding the change in DwrCU::parse_cu_header, i think Dwarf_ref will also return 0 when the attribute is not found, what i was trying to achieve in my tests was for stmt_list_offset to be kept at its default NO_STMT_LIST value if the attribute is absent.

You are right.
I made this fix:

   int64_t v;
   if (read_ref_attr(DW_AT_stmt_list, &v) == DW_DLV_OK)
     stmt_list_offset = v;


Thank you for review.
Thank you for contributing to gprofng.
Thank you for using gprofng.

-Vladimir


>
> Best regards,
>   Gilles

Re: gprofng SIGSEGV when processing unusual dwarf

[Gilles DUBOSCQ]

Great, Thanks!
 Gilles