Do you need to setup an Android device to repro these issues, or is there some way to run an ARM emulator? Are these tests using ARM?

Everyone I know runs tests on real hardware. Your regular phone works fine for this though, you only need to enable dev mode (go to settings, tap android build number 7 times, enable USB debugging.) If you've never done this, it might be a bit annoying, if you want I can investigate. If you want to give it a try:

https://chromium.googlesource.com/chromium/src/+/master/docs/android_debugging_instructions.md

and

              out/gnand4/bin/run_base_unittests  \
                  --gtest_filter=SafeNumerics.IntMinOperations  \
                  --test-arguments=--wait-for-debugger -t 6000
              build/android/adb_gdb --output-directory=out/gnand4 \
                  --target_arch=arm \
                  --package-name=org.chromium.native_test \
                  --adb=third_party/android_tools/sdk/platform-tools/adb \
                  --pid=  # fill in from `adb logcat -d | grep WaitForDebugger`


I had to change https://cs.chromium.org/chromium/src/base/debug/debugger.cc?q=WaitForDebugger&sq=package:chromium&dr=C&l=19 from DLOG to LOG (since I usually do release builds)

The bot uses this config: https://build.chromium.org/p/chromium.fyi/builders/ClangToTAndroidASan/builds/4559/steps/generate_build_files/logs/stdio ARM is the default target_cpu for target_os = "android", yes.

I have never done it, and I'm about to go on vacation tomorrow, so maybe I'm not the right person to chase this down. =/

I have this Nexus 4 on my desk, which I think was given to me specifically for the purpose of developing Chrome on Android. I should probably set it up. :)

A'ight, I'll take a look. Maybe you can look at  bug 668234 , that's what I originally wanted to do.

I can repro; asan not needed. args.gn:

target_os = "android"
is_clang = true
symbol_level = 1
is_debug = false
clang_use_chrome_plugins = false
#clang_base_path = getenv("HOME") + "/src/llvm-build"

Test passes with last line commented out, fails with it in.

It's due to 288024. That patch is harmless (http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20161128/407909.html), I guess it exposes an optimizer bug elsewhere :-(

Here's a single-file repro. Haven't managed to repro outside the chrome build system yet though.

Deleted: numerics.cc 67.5 KB

numerics.cc 67.5 KB View Download

I filed https://llvm.org/bugs/show_bug.cgi?id=31218 with a somewhat reduced repro (one-file at least)

James Molloy pointed out that this is due to undefined behavior in base/numerics's CheckedNumericState. It contains this code:

  template <typename Src>
  constexpr CheckedNumericState(Src value, bool is_valid)
      : value_(static_cast<T>(value)),
        is_valid_(is_valid && IsValueInRangeForNumericType<T>(value)) {
    static_assert(std::numeric_limits<Src>::is_specialized,
                  "Argument must be numeric.");
  }

Src is float, T is uint8_t, and value is out of range for a uint8_6. is_valid_ is computed correctly, but static_cast<uint8_t>(huge_float) is executed unconditionally, and this cast has undefined behavior according to [conf.fpint]p1:

   """A prvalue of a floating point type can be converted to a prvalue of an integer type. The conversion truncates;
that is, the fractional part is discarded. The behavior is undefined if the truncated value cannot be
represented in the destination type."""

What happens in practice here is that the compiler merges the u8 value field and the u8 valid field into a single u16, and then does

  myu16 = value;

to set the value half which spills over float bits into the is_valid part.

The morally correct fix I think is to change the ctor to

  template <typename Src>
  constexpr CheckedNumericState(Src value, bool is_valid)
      : is_valid_(is_valid && IsValueInRangeForNumericType<T>(value)) {
    static_assert(std::numeric_limits<Src>::is_specialized,
                  "Argument must be numeric.");
    if (is_valid_)
      value_ = is_valid_ ? static_cast<T>(value) : 0;
  }

but the ctor is constexpr and we're not using C++14, so we can't put statements in the ctor body. Unless we want switch the order of the value_ and is_valid_ fields (which we probably don't want to do?) we're forced to do something like:

  template <typename Src>
  constexpr CheckedNumericState(Src value, bool is_valid)
      : value_(is_valid && IsValueInRangeForNumericType<T>(value)
                   ? static_cast<T>(value)
                   : 0),
        is_valid_(is_valid && IsValueInRangeForNumericType<T>(value)) {
    static_assert(std::numeric_limits<Src>::is_specialized,
                  "Argument must be numeric.");
  }

Maybe reordering value_ and is_valid_ is ok though, then the ctor is just

  template <typename Src>
  constexpr CheckedNumericState(Src value, bool is_valid)
      : is_valid_(is_valid && IsValueInRangeForNumericType<T>(value)),
        value_( static_cast<T>(value) ) {
    static_assert(std::numeric_limits<Src>::is_specialized,
                  "Argument must be numeric.");
  }


That file has a few other questionable static_casts I'll have to audit.

jschuh, do you have a preference over reordering these two fields or duplicating the validity computation?

https://codereview.chromium.org/2537773009/

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e5b63cc2f7896b3ed1e8d89c3cab0b077ac14cc9

commit e5b63cc2f7896b3ed1e8d89c3cab0b077ac14cc9
Author: thakis <thakis@chromium.org>
Date: Fri Dec 02 22:26:00 2016

Fix a bunch of undefined behavior in CheckedNumericState

CheckedNumericState<T, NUMERIC_INTEGER> would unconditionally cast
the src type to the dest int type.  If the source type was a floating
point type that didn't fit in the integer type, this was undefined
behavior.

So only cast if is_valid_ is true.  To be able to check that, move
is_valid_ in front of the value_ field (since these are constexpr
ctors and we don't have C++14 constexpr).

BUG= 669642 

Review-Url: https://codereview.chromium.org/2537773009
Cr-Commit-Position: refs/heads/master@{#436055}

[modify] https://crrev.com/e5b63cc2f7896b3ed1e8d89c3cab0b077ac14cc9/base/numerics/safe_math_impl.h

This should hopefully cycle green now. I tried to use ubsan to verify that all this safe_math code is good now and discovered that it isn't. I filed  bug 670885  for that -- ubsan finds a bunch of stuff, but it seems to be "harmless undefined behavior" (hah!), so I'll stop here.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2e147959b92ac70c0eb3e0568ae664d39614b68c

commit 2e147959b92ac70c0eb3e0568ae664d39614b68c
Author: thakis <thakis@chromium.org>
Date: Sat Dec 03 00:19:12 2016

Include -fsanitize=float-cast-overflow in is_ubsan=true builds.

Also add a comment about the current state of ubsan.

BUG= 669642 

Review-Url: https://codereview.chromium.org/2550593004
Cr-Commit-Position: refs/heads/master@{#436093}

[modify] https://crrev.com/2e147959b92ac70c0eb3e0568ae664d39614b68c/build/config/sanitizers/BUILD.gn

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c4c7a57c10a808a2eefcc605095fe40fb75b2650

commit c4c7a57c10a808a2eefcc605095fe40fb75b2650
Author: thakis <thakis@chromium.org>
Date: Wed Dec 21 23:11:23 2016

Revert of Include -fsanitize=float-cast-overflow in is_ubsan=true builds. (patchset #1 id:1 of https://codereview.chromium.org/2550593004/ )

Reason for revert:
Somewhat speculative; it looks like clusterfuzz might use this list and filed many many float overflow bugs (https://bugs.chromium.org/p/chromium/issues/list?can=2&q=%22Float-cast-overflow%22&x=m&y=releaseblock&cells=ids). While we should fix all those, doing so by letting clusterfuzz file close to a hundred bugs assigned to random people isn't the way we should go about it, and CF filing all this bugs wasn't intended when I wrote this CL.

Original issue's description:
> Include -fsanitize=float-cast-overflow in is_ubsan=true builds.
>
> Also add a comment about the current state of ubsan.
>
> BUG= 669642 
>
> Committed: https://crrev.com/2e147959b92ac70c0eb3e0568ae664d39614b68c
> Cr-Commit-Position: refs/heads/master@{#436093}

TBR=krasin@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 669642 

Review-Url: https://codereview.chromium.org/2598813002
Cr-Commit-Position: refs/heads/master@{#440258}

[modify] https://crrev.com/c4c7a57c10a808a2eefcc605095fe40fb75b2650/build/config/sanitizers/BUILD.gn