Re: [PATCH v3 3/7] gdb/dwarf: pass compilation directory to line header

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2022-04-28 11:48, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> +  /* COMP_DIR is the value of the DW_AT_comp_dir attribute of the compilation
> Simon> +     unit in the context of which we are reading this line header, or nullptr
> Simon> +     if not applicable.  */
> Simon> +  line_header (const char *comp_dir)
> Simon> +    : m_comp_dir (comp_dir != nullptr ? comp_dir : "")
> Simon> +  {}
> 
> I think a single-argument constructor should be 'explicit'.

Fixed.  I should also initialize the bitfield field offset_in_dwz, fixed
that too.

> 
> Simon> +  /* Compilation directory of the compilation unit in the context of which this
> Simon> +     line header is read.  */
> Simon> +  std::string m_comp_dir;
> 
> Is there a need to copy the value here?
> I didn't check in this case, but often in the DWARF reader, a string
> will come directly from some mapped section, and in that case there
> aren't lifetime issues -- and so it's fine to just store the pointer.

I thought there might be lifetime issues.

read_file_scope owns a file_and_directory object, which contains a
possibly computed comp dir.  That object's lifetime is read_file_scope,
but the line_header created there (by calling handle_DW_AT_stmt_list)
lives on, it is stored in dwarf2_cu::line_header.

However, I see that we call fnd.intern_comp_dir prior to that, so the
returned string should actually be one with objfile lifetime, it should
be fine.  I'll change it to a const char*, we'll see if it causes any
test to fail (I build with ASan, so it should point out any
use-after-free).

Thanks,

Simon