Skip to content
  1. Jun 01, 2020
  3. Apr 21, 2020
    • Pavel Labath's avatar
      [DWARFDebugLine] Check for errors when parsing v2 file/dir lists · c475856d
      Pavel Labath authored 
      Summary:
Without this we could silently accept an invalid prologue because the
default DataExtractor behavior is to return an empty string when
reaching the end of file. And empty string is also used to terminate
these lists.

This makes the parsing code slightly more complicated, but this
complexity will go away once the parser starts working with truncating
data extractors. The reason I am doing it this way is because without
this, the truncation would regress the quality of error messages (right
now, we produce bad error messages only near EOF, but truncation would
make everything behave as if it was near EOF).

Reviewers: dblaikie, probinson, jhenderson

Subscribers: hiraditya, MaskRay, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D77555
      c475856d
  5. Mar 02, 2020
  7. Feb 26, 2020
    • Pavel Labath's avatar
      Recommit "[DWARFDebugLine] Avoid dumping prologue members we did not parse" · ced45978
      Pavel Labath authored 
      The patch was reverted in 69da4003 because of test failures on windows.
The problem was the unpredictable order of some of the error messages,
which I've tried to strenghten in that patch.

It turns out this is not possible to do in verbose mode because there
the data is being writted as it is being parsed. No amount of flushing
(as I've done in the non-verbose mode) will help that. Indeed, even
without any buffering the warning messages can end in the middle of a
line in non-verbose mode.

In this patch, I have reverted the changes which tested the relative
position of the warning message, except for the messages about
unsupported initial length, which are the ones I really wanted to test,
and which do come out reasonably.

The original commit message was:

This patch if motivated by D74560, specifically the subthread about what
to print upon encountering reserved initial length values.

If the debug_line prologue has an unsupported version, we skip parsing
the rest of the data. If we encounter an reserved initial length field,
we don't even parse the version. However, we still print out all members
(with value 0) in the dump function.

This patch introduces early exits in the Prologue::dump function so that
we print only the fields that were parsed successfully. In case of an
unsupported version, we skip printing all subsequent prologue fields --
because we don't even know if this version has those fields. In case of a
reserved unit length, we don't print anything -- if the very first field
of the prologue is invalid, it's hard to say if we even have a prologue
to begin with.

Note that the user will still be able to see the invalid/reserved
initial length value in the error message. I've modified (reordered)
debug_line_invalid.test to show that the error message comes straight
after the debug_line offset. I've also added some flush() calls to the
dumping code to ensure this is the case in all situations (without that,
the warnings could get out of sync if the output was not a terminal -- I
guess this is why std::iostreams have the tie() function).

Reviewers: jhenderson, ikudrin, dblaikie

Subscribers: hiraditya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D75043
      ced45978
  9. Feb 25, 2020
    • Pavel Labath's avatar
      Revert "[DWARFDebugLine] Avoid dumping prologue members we did not parse" · 69da4003
      Pavel Labath authored 
      The changed test started failing on the windows bots. Reverting while I
investigate.

This reverts commit deb116ee.
      69da4003
    • Pavel Labath's avatar
      [DWARFDebugLine] Avoid dumping prologue members we did not parse · deb116ee
      Pavel Labath authored 
      Summary:
This patch if motivated by D74560, specifically the subthread about what
to print upon encountering reserved initial length values.

If the debug_line prologue has an unsupported version, we skip parsing
the rest of the data. If we encounter an reserved initial length field,
we don't even parse the version. However, we still print out all members
(with value 0) in the dump function.

This patch introduces early exits in the Prologue::dump function so that
we print only the fields that were parsed successfully. In case of an
unsupported version, we skip printing all subsequent prologue fields --
because we don't even know if this version has those fields. In case of a
reserved unit length, we don't print anything -- if the very first field
of the prologue is invalid, it's hard to say if we even have a prologue
to begin with.

Note that the user will still be able to see the invalid/reserved
initial length value in the error message. I've modified (reordered)
debug_line_invalid.test to show that the error message comes straight
after the debug_line offset. I've also added some flush() calls to the
dumping code to ensure this is the case in all situations (without that,
the warnings could get out of sync if the output was not a terminal -- I
guess this is why std::iostreams have the tie() function).

Reviewers: jhenderson, ikudrin, dblaikie

Subscribers: hiraditya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D75043
      deb116ee
  11. Feb 12, 2020
  13. Feb 11, 2020
  15. Feb 10, 2020
    • James Henderson's avatar
      [DebugInfo][test] Replace pre-canned binary test · 1dc62d03
      James Henderson authored 
      The DebugInfo/dwarfdump-invalid-line-table test used a pre-canned binary
generated by a fuzzer to demonstrate a bug fix. Unfortunately, the
binary is rigid and requires hand-editing if we change behaviour, such
as rejecting certain properties within it (as I plan on doing in another
change).

Rather than hand-edit the binary, I have replaced it with two tests. The
first tests the high-level code path from the debug line parser that
produces the same error as this test previously did, and the second is a
set of unit test cases that comprehensively cover the
FormValue::skipValue method, which in turn covers the area that the
original bug fix touched.

Reviewed by: MaskRay, dblaikie

Differential Revision: https://reviews.llvm.org/D74202
      1dc62d03
  17. Jan 29, 2020
  19. Jan 28, 2020
  21. Jan 27, 2020
  23. Jan 10, 2020
  25. Jan 03, 2020
  27. Jan 02, 2020
  29. Jul 23, 2019
  31. May 10, 2018
    • James Henderson's avatar
      [DWARF] Rework debug line parsing to use llvm::Error and callbacks · a3acf99e
      James Henderson authored 
      Reviewed by: dblaikie, JDevlieghere, espindola

Differential Revision: https://reviews.llvm.org/D44560

Summary:
The .debug_line parser previously reported errors by printing to stderr and
return false. This is not particularly helpful for clients of the library code,
as it prevents them from handling the errors in a manner based on the calling
context. This change switches to using llvm::Error and callbacks to indicate
what problems were detected during parsing, and has updated clients to handle
the errors in a location-specific manner. In general, this means that they
continue to do the same thing to external users. Below, I have outlined what
the known behaviour changes are, relating to this change.

There are two levels of "errors" in the new error mechanism, to broadly
distinguish between different fail states of the parser, since not every
failure will prevent parsing of the unit, or of subsequent unit. Malformed
table errors that prevent reading the remainder of the table (reported by
returning them) and other minor issues representing problems with parsing that
do not prevent attempting to continue reading the table (reported by calling a
specified callback funciton). The only example of this currently is when the
last sequence of a unit is unterminated. However, I think it would be good to
change the handling of unrecognised opcodes to report as minor issues as well,
rather than just printing to the stream if --verbose is used (this would be a
subsequent change however).

I have substantially extended the DwarfGenerator to be able to handle
custom-crafted .debug_line sections, allowing for comprehensive unit-testing
of the parser code. For now, I am just adding unit tests to cover the basic
error reporting, and positive cases, and do not currently intend to test every
part of the parser, although the framework should be sufficient to do so at a
later point.

Known behaviour changes:
  - The dump function in DWARFContext now does not attempt to read subsequent
  tables when searching for a specific offset, if the unit length field of a
  table before the specified offset is a reserved value.
  - getOrParseLineTable now returns a useful Error if an invalid offset is
  encountered, rather than simply a nullptr.
  - The parse functions no longer use `WithColor::warning` directly to report
  errors, allowing LLD to call its own warning function.
  - The existing parse error messages have been updated to not specifically
  include "warning" in their message, allowing consumers to determine what
  severity the problem is.
  - If the line table version field appears to have a value less than 2, an
  informative error is returned, instead of just false.
  - If the line table unit length field uses a reserved value, an informative
  error is returned, instead of just false.
  - Dumping of .debug_line.dwo sections is now implemented the same as regular
  .debug_line sections.
  - Verbose dumping of .debug_line[.dwo] sections now prints the prologue, if
  there is a prologue error, just like non-verbose dumping.

As a helper for the generator code, I have re-added emitInt64 to the
AsmPrinter code. This previously existed, but was removed way back in r100296,
presumably because it was dead at the time.

This change also requires a change to LLD, which will be committed separately.

llvm-svn: 331971
      a3acf99e