gprofng SIGSEGV when processing unusual dwarf
Checks
Commit Message
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
Comments
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
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 :)
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
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
@@ -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 ();
@@ -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;
@@ -254,6 +254,8 @@ public:
Dwr_type *put_dwr_type (Dwr_Tag *dwrTag);
};
+#define NO_STMT_LIST 0xffffffffffffffffULL
+
class DwrCU
{
public: