New issue
Advanced search Search tips

Issue 907115 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 30
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 701825



Sign in to add a comment

Make boringssl_arm_cpuinfo_fuzzer work in component build

Project Member Reported by mmoroz@chromium.org, Nov 20

Issue description

We're planning to switch to is_component_build=true for libFuzzer builds, and apparently we'll lose one fuzz target if we do so: https://cs.chromium.org/chromium/src/third_party/boringssl/BUILD.gn?l=218&rcl=ce1a297ba4e1e2127e688b993616e9dca2f3cd85

Would it be possible to make that target work with component builds?
 
Hrm. We've avoided exporting those functions from the shared library because otherwise the static linker can't drop them from our shared library builds.

As the name suggests, the code being fuzzed is only relevant for (32-bit) ARM and Linux (it's the /proc/cpuinfo parser), so it'd be nice to avoid shipping it elsewhere. It's built on other platforms because fuzzing infrastructure tends not to be ARM.

But I could see about making it a header library instead, which should satisfy both desires.
We are planning to make this switch in first week of December. Will it be possible to please make this target work in component build by then. Otherwise, can you add a "additional_configs = [ "//testing/libfuzzer:no_clusterfuzz" ]" by then. Thanks!
Status: Started (was: Assigned)
I suspect we'd get the no_clusterfuzz effects anyway by way of the build rule being conditioned on is_component_build, but this should do the trick:
https://boringssl-review.googlesource.com/c/boringssl/+/33285
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 21

The following revision refers to this bug:
  https://boringssl.googlesource.com/boringssl/+/4f746a9073333d35db49173a8dbdc076004fa7d1

commit 4f746a9073333d35db49173a8dbdc076004fa7d1
Author: David Benjamin <davidben@google.com>
Date: Wed Nov 21 00:46:57 2018

Move ARM cpuinfo functions to the header.

ClusterFuzz folks want to switch to a shared library build, so call into
these another way. The new setup isn't quite ideal because the real code
builds as C and now tests as C++, but it should work.

Bug:  chromium:907115 
Change-Id: Ia1ffc18832739b09fee21b84ee5d181e61feaa15
Reviewed-on: https://boringssl-review.googlesource.com/c/33285
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>

[modify] https://crrev.com/4f746a9073333d35db49173a8dbdc076004fa7d1/crypto/cpu-arm-linux_test.cc
[modify] https://crrev.com/4f746a9073333d35db49173a8dbdc076004fa7d1/crypto/cpu-arm-linux.h
[modify] https://crrev.com/4f746a9073333d35db49173a8dbdc076004fa7d1/crypto/cpu-arm-linux.c

Thanks, David! Would you also need to change BUILD.gn file not to exclude that fuzzer?
Yeah, I'll do that when rolling the change into Chromium.
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 27

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

commit b39cccfb917ee1fbab09d970bd41c4835e56c485
Author: David Benjamin <davidben@chromium.org>
Date: Tue Nov 27 19:40:53 2018

Roll src/third_party/boringssl/src f241a59dc..6965d2560

https://boringssl.googlesource.com/boringssl/+log/f241a59dcca617c5b9d9880a8a9fd92996a654be..6965d25602754bc419c5f757d008ba1f4da49ae4

The following commits have Chromium bugs associated:
  4f746a907 Move ARM cpuinfo functions to the header.
  09f5a040d No longer set CQ-Verified label on CQ success/failure.

This additionally includes some manual follow-up tweaks to BUILD.gn for
4f746a907.

Bug:  906576 ,  907115 
Change-Id: If066a9fdd873edbad773cc8db7a8aadd679f269b
Reviewed-on: https://chromium-review.googlesource.com/c/1347096
Commit-Queue: David Benjamin <davidben@chromium.org>
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611256}
[modify] https://crrev.com/b39cccfb917ee1fbab09d970bd41c4835e56c485/DEPS
[modify] https://crrev.com/b39cccfb917ee1fbab09d970bd41c4835e56c485/third_party/boringssl/BUILD.gn
[modify] https://crrev.com/b39cccfb917ee1fbab09d970bd41c4835e56c485/third_party/boringssl/ios-aarch64/crypto/chacha/chacha-armv8.S
[modify] https://crrev.com/b39cccfb917ee1fbab09d970bd41c4835e56c485/third_party/boringssl/ios-aarch64/crypto/fipsmodule/aesv8-armx64.S
[modify] https://crrev.com/b39cccfb917ee1fbab09d970bd41c4835e56c485/third_party/boringssl/ios-aarch64/crypto/fipsmodule/sha1-armv8.S
[modify] https://crrev.com/b39cccfb917ee1fbab09d970bd41c4835e56c485/third_party/boringssl/ios-aarch64/crypto/fipsmodule/sha256-armv8.S
[modify] https://crrev.com/b39cccfb917ee1fbab09d970bd41c4835e56c485/third_party/boringssl/ios-aarch64/crypto/fipsmodule/sha512-armv8.S
[modify] https://crrev.com/b39cccfb917ee1fbab09d970bd41c4835e56c485/third_party/boringssl/ios-arm/crypto/fipsmodule/aesv8-armx32.S
[modify] https://crrev.com/b39cccfb917ee1fbab09d970bd41c4835e56c485/third_party/boringssl/linux-aarch64/crypto/chacha/chacha-armv8.S
[modify] https://crrev.com/b39cccfb917ee1fbab09d970bd41c4835e56c485/third_party/boringssl/linux-aarch64/crypto/fipsmodule/aesv8-armx64.S
[modify] https://crrev.com/b39cccfb917ee1fbab09d970bd41c4835e56c485/third_party/boringssl/linux-aarch64/crypto/fipsmodule/sha1-armv8.S
[modify] https://crrev.com/b39cccfb917ee1fbab09d970bd41c4835e56c485/third_party/boringssl/linux-aarch64/crypto/fipsmodule/sha256-armv8.S
[modify] https://crrev.com/b39cccfb917ee1fbab09d970bd41c4835e56c485/third_party/boringssl/linux-aarch64/crypto/fipsmodule/sha512-armv8.S
[modify] https://crrev.com/b39cccfb917ee1fbab09d970bd41c4835e56c485/third_party/boringssl/linux-arm/crypto/fipsmodule/aesv8-armx32.S

Status: Fixed (was: Started)
Should be sorted out.

Sign in to add a comment