1. 10 May, 2021 1 commit
  2. 23 Mar, 2021 1 commit
  3. 22 Mar, 2021 2 commits
  4. 16 Mar, 2021 1 commit
  5. 12 Mar, 2021 1 commit
    • Clemens Backes's avatar
      [logging] Fix printing of single-byte enums · d8c8387a
      Clemens Backes authored
      We still get e.g. ClusterFuzz reports with enums printed as
      non-printable single-character strings (see linked bug).
      This CL fixes this, and also includes the integral enum value for enum
      that come with their own output operator.
      
      This makes error messages strictly better, at the cost of some more code
      per enum which is being used in a CHECK/DCHECK.
      Note that binary size of release builds is not affected, since we do not
      print the values there.
      
      R=nicohartmann@chromium.org
      
      Bug: v8:11384, chromium:1187484
      Change-Id: I066b32f68440096babed9b629c7ffe3f2285cba8
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2756226Reviewed-by: 's avatarNico Hartmann <nicohartmann@chromium.org>
      Commit-Queue: Clemens Backes <clemensb@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#73373}
      d8c8387a
  6. 18 Jun, 2020 1 commit
  7. 10 Jan, 2020 1 commit
    • Clemens Backes's avatar
      [base] Improve logging for long error messages · 193c08ad
      Clemens Backes authored
      When comparing objects which get printed to very long strings (e.g.
      collections like vectors), it's much more readable if they get printed
      to individual lines. Differences are much easier to spot then.
      
      This CL refactors the CHECK/DCHECK macros to print the left hand side
      and right-hand side in individual lines if any of them is longer than 50
      characters.
      
      To that end, the {PrintCheckOperand} method (only used from
      {MakeCheckOpString}) is changed to return the string directly instead of
      printing to an output stream.
      
      R=mlippautz@chromium.org
      
      Change-Id: I6e24a5cbfeb1af53fa0aca2828e23f642b15569c
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1991866Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
      Commit-Queue: Clemens Backes <clemensb@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#65705}
      193c08ad
  8. 12 Nov, 2019 1 commit
  9. 13 May, 2019 1 commit
  10. 10 May, 2019 1 commit
  11. 20 Sep, 2018 1 commit
  12. 28 Feb, 2018 1 commit
    • Nico Weber's avatar
      Make v8 build with -Wmicrosoft-cast under clang-cl. · 58b386c4
      Nico Weber authored
      gcc and clang (and the standard) don't allow implicit conversion of
      function pointers to object pointers. MSVC does allow that, and since
      system headers require this to work, clang-cl allows it too -- but
      it emits a -Wmicrosoft-cast warning (which we currently suppress in
      the Chromium build, but which we want to enable.)
      
      As a side effect, when printing a function pointer to a stream, MSVC
      (and clang-cl) will pick the operator<<(void*) overload, while gcc
      and clang will pick operator<<(bool) since the best allowed conversion
      they find is from function pointer to bool.
      
      To prevent the clang-cl warning, we need to make sure that we never
      directly print a function pointer to a stream. In v8, this requires
      two changes:
      
      1. Give PrintCheckOperand() an explicit specialization for function
         pointers and explicitly cast to void* there.  This ports
         https://codereview.chromium.org/2515283002/ to V8, and also fixes a
         bug on non-Windows where DCHECK() of function pointers would print
         "(1 vs 1)" instead of the function's addresses.
         (The bug remains with member function pointers,
         where it's not clear what to print instead of the 1.)
      
      2. has_output_operator<T> must not use operator<< on its argument
         in an evaluated context if T is a function pointer.  This patch
         modifies has_output_operator<> to use an unevaluated context instead,
         which is simpler than the current approach (and matches what Chromium's
         base does), but changes behavior    in minor (boring) ways
         (see template-utils-unittest.cc), since operator<<() is now
         called with a temporary and only operator<<() implementations callable
         with a temporary are considered.
         A more complicated but behavior-preserving alternative would be to
         add an explicit specialization for function pointers. You can see
         this variant in patch set 1 on gerrit.
      
      Bug: chromium:550065
      Change-Id: Idc2854d6c258b7fc0b959604006d8952a79eca3d
      Reviewed-on: https://chromium-review.googlesource.com/940004
      Commit-Queue: Nico Weber <thakis@chromium.org>
      Reviewed-by: 's avatarClemens Hammacher <clemensh@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#51636}
      58b386c4
  13. 13 Feb, 2018 1 commit
  14. 19 Dec, 2017 1 commit
  15. 16 Oct, 2017 1 commit
  16. 21 Sep, 2017 1 commit
    • Clemens Hammacher's avatar
      [base] Allow comparing enums in (D)CHECKs · 3a063911
      Clemens Hammacher authored
      In the current implementation, compilation would fail because
      operator<< is not defined for enum classes. For others, the compiler
      finds more than one operator<<, so it fails because it's ambiguous.
      
      This CL fixes this by printing the integer value for enums, uses the
      operator<< for all values that support it, and prints "<unprintable>"
      otherwise.
      
      Also, lots of unit tests.
      
      R=ishell@chromium.org
      
      Bug: v8:6837
      Change-Id: I895ed226672aa07213f9605e094b87af186ec2e4
      Reviewed-on: https://chromium-review.googlesource.com/671016
      Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
      Reviewed-by: 's avatarIgor Sheludko <ishell@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#48110}
      3a063911
  17. 15 Sep, 2017 1 commit
  18. 31 Aug, 2017 1 commit
  19. 28 Aug, 2017 1 commit
  20. 24 Aug, 2017 1 commit
  21. 07 Aug, 2017 1 commit
    • Clemens Hammacher's avatar
      Move helper struct from logging.h to template-utils.h · 84dc3679
      Clemens Hammacher authored
      I want to reuse the PassType helper in another CL, thus move it from
      logging.h to template-utils.h, and rename it to pass_value_or_ref to
      match other helpers there.
      Also, add a boolean template parameter to declare whether array
      dimensions should be removed. The default is to do so, which helps to
      reduce the number of template instantiations by always passing arrays
      as pointers.
      
      Also, fix the usages in logging.h to actually use that helper when
      instantiating other template functions. This will reduce the number of
      instantiations.
      
      And finally, we now have unit tests for the template utils, to document
      what we expect, and test that this works on all architectures.
      
      R=ishell@chromium.org, tebbi@chromium.org
      
      Change-Id: I1ef5d2a489a5cfc7601c5ab13748674e3aa86cd6
      Reviewed-on: https://chromium-review.googlesource.com/594247
      Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
      Reviewed-by: 's avatarTobias Tebbi <tebbi@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#47191}
      84dc3679
  22. 12 Jul, 2017 1 commit
  23. 26 Jun, 2017 1 commit
  24. 22 Jun, 2017 1 commit
    • Clemens Hammacher's avatar
      [cleanup] extern "C" not needed for V8_Fatal · 696f31ba
      Clemens Hammacher authored
      Why I want to fix this: I got a CL to replace V8_NORETURN by
      [[noreturn]], but clang-format formats this as
        extern "C"[[noreturn]] PRINT_FORMAT...
      (i.e. missing whitespace).
      Also, this is the only extern "C" function in our code base, so if we
      do not need to call it from C, we should just get rid of it.
      
      R=jochen@chromium.org
      BUG=v8:6474
      
      Change-Id: I950bdc505822eb37a107c58e63c82a61907ba515
      Reviewed-on: https://chromium-review.googlesource.com/539341Reviewed-by: 's avatarJochen Eisinger <jochen@chromium.org>
      Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#46149}
      696f31ba
  25. 13 Jun, 2017 1 commit
  26. 31 May, 2017 1 commit
    • Clemens Hammacher's avatar
      [logging] Print const char* as pointer value · 63c4cd96
      Clemens Hammacher authored
      When checking {const char*} (or similar) against each other, don't
      print them as c strings on failure. Just print the pointer value.
      In wasm, where we use byte pointers into wasm wire bytes, this was
      sometimes hiding check failures behind segfaults which happened when
      trying to output invalid pointers as c strings.
      Anyway, it's more useful to see the raw pointer values in these cases.
      Other use cases, where we really compare against c string pointers
      should be rare in our code base.
      
      R=ishell@chromium.org
      
      Change-Id: I92a13221d18c987a97cf2a29ac8f454178ff2bb5
      Reviewed-on: https://chromium-review.googlesource.com/517166
      Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
      Reviewed-by: 's avatarIgor Sheludko <ishell@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#45642}
      63c4cd96
  27. 30 May, 2017 2 commits
  28. 12 May, 2017 1 commit
  29. 17 Mar, 2017 1 commit
  30. 20 Jan, 2017 1 commit
    • clemensh's avatar
      Fix CHECK_OP implementation in Release builds · 4dcbe86e
      clemensh authored
      In this particular case, we just did a (lhs)op(rhs), ignoring the case
      that lhs and rhs might have different signedness.
      This CL changes that to use the proper Cmp##op##Impl implementation,
      which does two comparisions for signed-vs-unsigned checks, avoiding
      compiler errors.
      
      R=ishell@chromium.org
      
      Review-Url: https://codereview.chromium.org/2642383002
      Cr-Commit-Position: refs/heads/master@{#42566}
      4dcbe86e
  31. 01 Dec, 2016 1 commit
    • clemensh's avatar
      [base] Define CHECK comparison for signed vs. unsigned · db0c86fa
      clemensh authored
      The current CHECK/DCHECK implementation fails statically if a signed
      value is compared against an unsigned value. The common solution is to
      cast on each caller, which is tedious and error-prone (might hide bugs).
      This CL implements signed vs. unsigned comparisons by executing up to
      two comparisons. For example, if i is int32_t and u is uint_32_t, a
      DCHECK_LE(i, u) would create the check
      i <= 0 || static_cast<uint32_t>(i) <= u.
      For checks against constants, at least one of the checks can be removed
      by compiler optimizations.
      
      The tradeoff we have to make is to sometimes silently execute an
      additional comparison. And we increase code complexity of course, even
      though the usage is just as easy (or even easier) as before.
      
      The compile time impact seems to be minimal:
      I ran 3 full compilations for Optdebug on my local machine, one time on
      the current ToT, one time with this CL plus http://crrev.com/2524093002.
      Before: 143.72 +- 1.21 seconds
      Now: 144.18 +- 0.67 seconds
      
      In order to check that the new comparisons are working, I refactored
      some DCHECKs in wasm to use the new magic, and added unit test cases.
      
      R=ishell@chromium.org, titzer@chromium.org
      CC=ahaas@chromium.org, bmeurer@chromium.org
      
      Committed: https://crrev.com/5925074a9dab5a8577766545b91b62f2c531d3dc
      Review-Url: https://codereview.chromium.org/2526783002
      Cr-Original-Commit-Position: refs/heads/master@{#41275}
      Cr-Commit-Position: refs/heads/master@{#41411}
      db0c86fa
  32. 29 Nov, 2016 1 commit
  33. 24 Nov, 2016 4 commits
    • clemensh's avatar
      Revert of [base] Pass scalar arguments by value in CHECK/DCHECK (patchset #3... · 29ee6244
      clemensh authored
      Revert of [base] Pass scalar arguments by value in CHECK/DCHECK (patchset #3 id:40001 of https://codereview.chromium.org/2524093002/ )
      
      Reason for revert:
      Seems to cause compile errors on Android. Will investigate on Monday.
      
      Original issue's description:
      > [base] Pass scalar arguments by value in CHECK/DCHECK
      >
      > This not only potentially improves performance, but also avoids weird
      > linker errors, like the one below, where I used Smi::kMinValue in a
      > DCHECK_EQ.
      >
      > > [421/649] LINK ./mksnapshot
      > > FAILED: mksnapshot
      > > src/base/logging.h|178| error: undefined reference to
      >   'v8::internal::Smi::kMinValue'
      >
      > R=bmeurer@chromium.org, ishell@chromium.org
      >
      > Committed: https://crrev.com/76723502528c5af003fdffc3520632ea2a13fef3
      > Cr-Commit-Position: refs/heads/master@{#41273}
      
      TBR=bmeurer@chromium.org,ishell@chromium.org
      # Skipping CQ checks because original CL landed less than 1 days ago.
      NOPRESUBMIT=true
      NOTREECHECKS=true
      NOTRY=true
      
      Review-Url: https://codereview.chromium.org/2527883004
      Cr-Commit-Position: refs/heads/master@{#41278}
      29ee6244
    • clemensh's avatar
      Revert of [base] Define CHECK comparison for signed vs. unsigned (patchset #5... · 0406620c
      clemensh authored
      Revert of [base] Define CHECK comparison for signed vs. unsigned (patchset #5 id:80001 of https://codereview.chromium.org/2526783002/ )
      
      Reason for revert:
      Need to revert previous CL because of Android compile error, and this one depends in it.
      
      Original issue's description:
      > [base] Define CHECK comparison for signed vs. unsigned
      >
      > The current CHECK/DCHECK implementation fails statically if a signed
      > value is compared against an unsigned value. The common solution is to
      > cast on each caller, which is tedious and error-prone (might hide bugs).
      > This CL implements signed vs. unsigned comparisons by executing up to
      > two comparisons. For example, if i is int32_t and u is uint_32_t, a
      > DCHECK_LE(i, u) would create the check
      > i <= 0 || static_cast<uint32_t>(i) <= u.
      > For checks against constants, at least one of the checks can be removed
      > by compiler optimizations.
      >
      > The tradeoff we have to make is to sometimes silently execute an
      > additional comparison. And we increase code complexity of course, even
      > though the usage is just as easy (or even easier) as before.
      >
      > The compile time impact seems to be minimal:
      > I ran 3 full compilations for Optdebug on my local machine, one time on
      > the current ToT, one time with this CL plus http://crrev.com/2524093002.
      > Before: 143.72 +- 1.21 seconds
      > Now: 144.18 +- 0.67 seconds
      >
      > In order to check that the new comparisons are working, I refactored
      > some DCHECKs in wasm to use the new magic.
      >
      > R=bmeurer@chromium.org, titzer@chromium.org
      >
      > Committed: https://crrev.com/5925074a9dab5a8577766545b91b62f2c531d3dc
      > Cr-Commit-Position: refs/heads/master@{#41275}
      
      TBR=ishell@chromium.org,titzer@chromium.org
      # Skipping CQ checks because original CL landed less than 1 days ago.
      NOPRESUBMIT=true
      NOTREECHECKS=true
      NOTRY=true
      
      Review-Url: https://codereview.chromium.org/2531533003
      Cr-Commit-Position: refs/heads/master@{#41277}
      0406620c
    • clemensh's avatar
      [base] Define CHECK comparison for signed vs. unsigned · 5925074a
      clemensh authored
      The current CHECK/DCHECK implementation fails statically if a signed
      value is compared against an unsigned value. The common solution is to
      cast on each caller, which is tedious and error-prone (might hide bugs).
      This CL implements signed vs. unsigned comparisons by executing up to
      two comparisons. For example, if i is int32_t and u is uint_32_t, a
      DCHECK_LE(i, u) would create the check
      i <= 0 || static_cast<uint32_t>(i) <= u.
      For checks against constants, at least one of the checks can be removed
      by compiler optimizations.
      
      The tradeoff we have to make is to sometimes silently execute an
      additional comparison. And we increase code complexity of course, even
      though the usage is just as easy (or even easier) as before.
      
      The compile time impact seems to be minimal:
      I ran 3 full compilations for Optdebug on my local machine, one time on
      the current ToT, one time with this CL plus http://crrev.com/2524093002.
      Before: 143.72 +- 1.21 seconds
      Now: 144.18 +- 0.67 seconds
      
      In order to check that the new comparisons are working, I refactored
      some DCHECKs in wasm to use the new magic.
      
      R=bmeurer@chromium.org, titzer@chromium.org
      
      Review-Url: https://codereview.chromium.org/2526783002
      Cr-Commit-Position: refs/heads/master@{#41275}
      5925074a
    • clemensh's avatar
      [base] Pass scalar arguments by value in CHECK/DCHECK · 76723502
      clemensh authored
      This not only potentially improves performance, but also avoids weird
      linker errors, like the one below, where I used Smi::kMinValue in a
      DCHECK_EQ.
      
      > [421/649] LINK ./mksnapshot
      > FAILED: mksnapshot
      > src/base/logging.h|178| error: undefined reference to
        'v8::internal::Smi::kMinValue'
      
      R=bmeurer@chromium.org, ishell@chromium.org
      
      Review-Url: https://codereview.chromium.org/2524093002
      Cr-Commit-Position: refs/heads/master@{#41273}
      76723502
  34. 23 Nov, 2016 1 commit
  35. 07 Oct, 2016 1 commit