The DWARFv2-4 specification for the line table header states that the include directories and file name tables both end with a single null byte. Prior to this change, the parser did not detect if this byte was missing, because it also stopped reading the tables once it reached the prologue end, as claimed by the header_length field. This change adds a check that the terminator has been seen at the end of each table.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp | ||
---|---|---|
165–171 | Perhaps this'd be a little more terse (but not necessarily better) written as: Terminated = S.empty(); if (Terminated) break; or maybe if ((Terminated = S.empty())) break; Not necessary if you prefer the current version. I'd probably more strongly suggest making the variable names a bit simpler - probably just using "Terminated" for both. | |
178 | maybe make it a bit more explicit by saying "null terminated"? |
llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp | ||
---|---|---|
165–171 | Terminated +1 The variable IncludeDirectories below suggests what Terminated means. Both the current form and if ((Terminated = S.empty())) break; look fine to me. |
llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp | ||
---|---|---|
165–171 | I think I marginally prefer the current form for readability reasons, so I'll leave that as is. The renaming makese sense though. I considered making that change before putting it up for review. | |
178 | That's fine with me, although I should point out that the strings themselves are always null terminated, so even if the null terminator byte is missing, the table will still have a null byte at the end. |
Perhaps this'd be a little more terse (but not necessarily better) written as:
or maybe
Not necessary if you prefer the current version.
I'd probably more strongly suggest making the variable names a bit simpler - probably just using "Terminated" for both.