Commit graph

57 commits

Author SHA1 Message Date
George Burgess IV
2f7876596e fortify: remove 'optimizations' for functions that LLVM knows about
Over the last year, LLVM apparently learned how to optimize many
FORTIFY'ed functions. I went through the list of functions it optimizes,
and simplified their implementations here.

This is more than a code health thing; __bos_trivially_ge expands to a
branch that's not eliminated until after inlining, so it can actually
cause some functions (like one of std::string's ctors) to become
uninlineable.

Bug: 148189733
Test: hand-checked the IR we get for each of the changed functions. Many
      get optimized to their non-_chk variant when appropriate. Others
      will get optimized to non-_chk versions when bos == -1. Bug repro
      also now shows all 'inline's.

Change-Id: Ic360818ad9daaeda3958e1282af41087f85122a3
2020-02-04 21:52:36 -08:00
Logan Chien
d98dac41df versioner: Fix fortified sendto
This commit fixes an error in fortified `sendto` function.  Since
`__sendto_chk` is only introduced in API 26, the usage should be guarded
with `__ANDROID__API__ >= 26` instead of
`__ANDROID_API__ >= __ANDROID_API_N_MR1__` (25).

Bug: 118991081
Test: source development/vndk/tools/header-checker/android/envsetup.sh && \
      source build/envsetup.sh && \
      lunch aosp_arm64-userdebug && \
      m versioner && \
      ./bionic/tools/versioner/run_tests.py
Change-Id: Ibc08244645c3fe76a72d0107138f67ffd56f5caa
2019-12-23 07:14:05 -08:00
Elliott Hughes
95c6cd753f Stop using the __ANDROID_API_x__ constants.
Historically we've made a few mistakes where they haven't matched the
right number. And most non-Googlers are much more familiar with the
numbers, so it seems to make sense to rely more on them. Especially in
header files, which we actually expect real people to have to read from
time to time.

Test: treehugger
Change-Id: I0d4a97454ee108de1d32f21df285315c5488d886
2019-12-20 13:26:14 -08:00
George Burgess IV
8a0cdb15a1 fortify: allow diagnostics without run-time checks (attempt #2)
In configs like ASAN, we can't use _chk functions. This CL builds off of
previous work to allow us to still emit diagnostics in conditions like
these.

Wasn't 100% sure what a good test story would look like here. Opinions
appreciated.

Bug: 141267932
Test: checkbuild on internal-master. TreeHugger for x86_64.
Change-Id: I65da9ecc9903d51a09f740e38ab413b9beaeed88
2019-11-19 14:20:26 -08:00
George Burgess IV
f359548bf1 Revert "fortify: allow diagnostics without run-time checks"
This reverts commit d7e11b8853.

Reason for revert: Breaks aosp_x86_64-eng. Will look into it and
unbreak when it's not almost midnight. :)

Change-Id: I21f76efe4d19c70d0b14630e441376d359a45b49
2019-11-07 07:07:38 +00:00
George Burgess IV
d7e11b8853 fortify: allow diagnostics without run-time checks
In configs like ASAN, we can't use _chk functions. This CL builds off of
previous work to allow us to still emit diagnostics in conditions like
these.

Wasn't 100% sure what a good test story would look like here. Opinions
appreciated.

Bug: 141267932
Test: checkbuild on internal-master
Change-Id: I8d4f77d7b086a8128a18a0a0389243d7fa05b00f
2019-11-01 16:25:55 -07:00
George Burgess IV
3aedee9828 fortify: remove last uses of __bos_trivially_not*
Since we're using the gt/ge ones a lot now, having `not` versions
probably just adds to confusion. Swap out their remaining uses and
delete them.

Bug: 141267932
Test: m checkbuild on internal-master
Change-Id: I2107ae65007a4995e4fa23371fefe4db7547f43b
2019-10-07 16:18:44 -07:00
George Burgess IV
fd1ff4b2c8 fortify(string): emit diagnostics regardless of API level
Bug: 141267932
Test: m checkbuild on internal-master

Change-Id: I7dc10b56b667f8f2f2cd360b008d834af92a72e4
2019-10-07 16:18:44 -07:00
George Burgess IV
9349b9e51b fortify(fcntl): emit diagnostics regardless of API level
Bug: 141267932
Test: m checkbuild on internal-master

Change-Id: I2037548cc2061e46c379931588194c21dfe234b4
2019-10-07 16:18:41 -07:00
Treehugger Robot
3e565bd7ee Merge "fortify(unistd): emit diagnostics regardless of API level" 2019-10-04 06:36:54 +00:00
Elliott Hughes
0e14c5a869 <unistd.h> __INTRODUCED_IN cleaup.
The NDK doesn't support anything older than API level 16, so remove some
more clutter.

Test: builds
Change-Id: If257a27841396af001b089b7ae0fbd8c3e0128e4
2019-10-03 20:35:38 -07:00
George Burgess IV
da8d30f357 fortify(unistd): emit diagnostics regardless of API level
Bug: 141267932
Test: m checkbuild on internal-master

Change-Id: I17ad1283cbedaec2ab070134320e1c5401d3e6a4
2019-10-03 20:30:02 -07:00
George Burgess IV
113d6fa14e fortify(stdio): emit diagnostics regardless of API level
Bug: 141267932
Test: m checkbuild on internal-master

Change-Id: I875a072c74f578d4404576c5bb42fd8ea30ff68d
2019-09-30 13:00:36 -07:00
George Burgess IV
db87607524 fortify(poll): emit diagnostics regardless of API level
Bug: 141267932
Test: m checkbuild on internal-master

Change-Id: I5e147d28ec606c104b5905fd3d0c3fc197f0e4b8
2019-09-30 13:00:36 -07:00
George Burgess IV
bc37c5a8ce fortify(strings): emit diagnostics regardless of API level
Bug: 141267932
Test: m checkbuild on internal-master

Change-Id: Ief185320f93ec31f4d0a8035c450a7ab7b4e02b4
2019-09-30 13:00:35 -07:00
George Burgess IV
d0a0f72852 fortify(socket/stat): emit diagnostics regardless of API level
Bug: 141267932
Test: m checkbuild on internal-master

Change-Id: I7fe29184290e19ec080b204aa98d513ce59bc66b
2019-09-30 13:00:35 -07:00
George Burgess IV
36926f4ca8 fortify: account for new clang diagnostics
Clang recently grew its own diagnostics for memcpy and such. These are
generally higher-quality than what we can do with diagnose_if, since
clang is happy to include e.g., sizes of things per-callsite. Move to
those instead where applicable.

Bug: 131861088, 123644155
Test: blueline internal-master checkbuild; treehugger
Change-Id: I701f5a8b247ba2948ca47fdc60ff5198b564c03e
2019-09-16 12:48:04 -07:00
Logan Chien
b33952c777 versioner: Fix bzero/bcopy fortify
This commit replaces `bzero` with `__bionic_bzero` and `bcopy` with
`__bionic_bcopy` because `bzero` and `bcopy` are partially defined in
`libc.map.txt`.  Bionic versioner raises errors because versioner treats
static inline functions as exported function definitions then it
compares the availability with the information specified in
`libc.map.txt`.

This commit fixes the problem by replacing static inline functions into
`__bionic_{bzero,bcopy}` and defining aliases for source-level
compatibility.

Test: PATH=$(pwd)/prebuilts/clang-tools/linux-x86/bin:$PATH \
      bionic/tools/versioner/run_tests.py
Bug: 140110040

Change-Id: I97f2f0dc0abccd0a9fcfe5bb02f4e918362d35cc
2019-08-30 10:17:36 -07:00
George Burgess IV
261b7f4867 fortify: replace bzero/bcmp defines
__builtin_*_chk will emit warnings when things are trivially broken.
Emitting errors instead is probably better (and we can be a bit smarter
about how we emit code for trivially safe cases.)

Bug: 131861088
Test: checkbuild + bionic-unit-tests on blueline
Change-Id: I33957ad419922d0760304758ecb9bc8ad33e0b64
2019-06-20 13:05:23 -07:00
George Burgess IV
849c0b9f51 fortify: add __mempcpy_chk
Bug: 131861088
Test: mma + bionic-unit-tests on blueline
Change-Id: I02f8f87d5db0ba5fecec410da32f6ffa2c98ef57
2019-06-13 23:29:37 -07:00
George Burgess IV
74519e7aa5 fortify: fix overflow checks in unistd
We should only be calling _real versions of the functions that use this
if the input size is verifiably <= SSIZE_MAX. Otherwise, just fall
through to _chk and let that handle it.

Bug: 131861088
Test: mma && bionic-unit-tests
Change-Id: Iba04e486ef91ea1b3539ab6df6260429264e66b4
2019-06-13 23:26:02 -07:00
George Burgess IV
26d25a22e2 fortify: add even more warnings
Bug: 131861088
Test: mma
Change-Id: I557309b3e25b54321ee1fe0207f18b6e840bf76e
2019-06-13 23:26:02 -07:00
George Burgess IV
8c0ec114c5 fortify: fix up a few diagnostics; add __wur to realpath
As it says on the box.

Since realpath isn't a function definition, any attributes it provides
here just add to the "regular" realpath.

__wur is being added to realpath because it returns NULL on failure, and
the contents of the input buffer are undefined in that case. A blueline
checkbuild showed 0 complaints about this new __wur, so it seems
harmless to add.

Bug: 131861088
Test: mma
Change-Id: If5f47e0e290d86df69c0888711e29775c390fca4
2019-06-13 23:26:02 -07:00
George Burgess IV
2356c93d39 fortify: add bit checking for open(at)?64 functions
This also adds _2-variants for these functions, for extra glorious
checking

Bug: 131861088
Test: mma
Change-Id: I80475ff4bb220c0c47894e9532426f53412f176f
2019-06-13 23:26:02 -07:00
George Burgess IV
77f99aaf58 fortify: add diagnostics for str* functions
This CL allows us to diagnose string functions that get an explicit size
passed into them, and string functions that are trivially misused.

Bug: 131861088
Test: mma
Change-Id: I894aec99420a75c6474cfd7d5010f0cf2f10ab21
2019-06-13 23:26:02 -07:00
George Burgess IV
c03d5964d0 fortify: s/([gl])eq/\1e/g
Follow-up from review comments in
https://android-review.googlesource.com/c/platform/bionic/+/961600

Bug: 131861088
Test: mma
Change-Id: Ic2d48c935ced3c7e875923810f4e9970e7439e51
2019-05-23 15:22:01 -07:00
George Burgess IV
d9865e7734 fortify: more use of __builtin_constant_p
This converts all of stdio to short-circuit _chk functions in trivially
safe cases.

Bug: 131861088
Test: checkbuild on internal master. blueline bionic tests pass + it
      boots.
Change-Id: I981ac9bd19112492d5a47dc5277526426b9af710
2019-05-23 15:01:55 -07:00
George Burgess IV
a1a09b211e fortify: use __builtin_constant_p for more short-circuits
This also lets us retire our |__enable_if| version of |strlen|, which
should catch strictly fewer cases where we can fold the string's length
to a constant than |__builtin_constant_p| inside of |strlen|.

Bug: 131861088
Test: checkbuild on internal master. blueline bionic tests pass + it
      boots.
Change-Id: I21b750a24f7d1825591a88d12a385be03a0a7ca3
2019-05-23 15:01:54 -07:00
George Burgess IV
5da5dd5215 fortify: Migrate trivial cases to dynamic check macros
|__builtin_constant_p| has become more flexible in clang. In particular,
it's no longer forcibly lowered before inlining, so we can actually use
it on function parameters (or |__bos(param)|).

This CL tweaks things so that trivially safe calls to FORTIFY'ed
functions compile into direct calls to those functions, rather than to
their _chk counterparts. This will be the most impactful with things
like |memset|, |memcpy|, etc., since clang has way more flexibility
about how to lower those than it does with |__memset_chk|,
|__memcpy_chk|, ...

As noted in the comments, the spelling of the new macros is meant to
match closely with the spelling of our |__bos_static| macros used in
|diagnose_if|.

This isn't a full cleanup of all of the cases in which we can do this.
Just a start on the super simple cases.

Bug: 131861088
Test: m checkbuild; blueline boots.

Change-Id: I696f42ce4a65231e0c4a78a4c5133a6be1cb7708
2019-05-23 13:39:04 -07:00
George Burgess IV
ff7179350a fortify: inline #defined strings
These were originally #defined so we could share them between our
GCC and clang FORTIFY implementations. Since we no longer have a GCC
FORTIFY, #defining them is sort of pointless.

Bug: 131861088
Test: mma
Change-Id: I2ae4e0bdebbed16c946f5df7cc38c471881b481e
2019-05-23 13:38:09 -07:00
George Burgess IV
5273dc588a fortify: use a macro in diagnose_if
Our diagnose_if conditions are repetitive. It's potentially convenient
to hide that behind a macro. There's an upcoming refactor to our
run-time checks; having static checks look super similar is convenient,
and makes correctness (hopefully) slightly more obvious.

Bug: 131861088
Test: checkbuild on internal master.
Change-Id: Ic39a3b6bf020734c1bef6be144f61ef81466aafe
2019-05-23 13:38:09 -07:00
Elliott Hughes
0d1a8a5b0b Reland "Retire GCC FORTIFY."
This reverts commit 4dbe8fc22f.

The angler/bullhead builds are now dead, so this can go in.

Bug: https://issuetracker.google.com/74404306
Change-Id: I130cdcd6375b6125cb8a1e63a4a540b0dbe41ceb
2018-07-24 19:36:51 +00:00
Elliott Hughes
4dbe8fc22f Revert "Retire GCC FORTIFY."
This reverts commit 067bdc9546.

The angler/bullhead builds still use GCC, so I'll wait until they're turned down.

Bug: https://issuetracker.google.com/74404306
Change-Id: I777b08dd6ab5250d532b4b5cc56e9a790c13db20
2018-07-24 18:54:16 +00:00
Elliott Hughes
067bdc9546 Retire GCC FORTIFY.
NDK r18 (which removes GCC) is about to ship. We don't need this any more.

The immediate motivation for removing this right now is that the test
fails whenever we add Clang-only flags to the build system.

While we're here, clean up <stdatomic.h> too.

Bug: https://issuetracker.google.com/74404306
Test: ran tests
Change-Id: Iaad5d634d1ba76f0b6f19ad32cc27b2533771a4a
2018-07-23 17:22:58 -07:00
Elliott Hughes
b83bf14a35 Fortify ppoll64.
I've deliberately not bothered with the GCC implementation because we'll
have removed GCC from the NDK before anyone gets to use this.

Bug: http://b/72493232
Test: ran tests
Change-Id: Icfc2a3b214739ab53aa41bacacc11b5c67498fb4
2018-03-22 13:59:02 -07:00
Chih-Hung Hsieh
f81abef148 Use __BIONIC_FORTIFY_VARIADIC for variadic functions.
* Variadic functions usually cannot be inlined.
* Do not use misleading __always_inline attribute,
  and also avoid early clang 7.0 compiler bug.

Bug: 72412382
Test: build and boot aosp*-eng in emulator
Change-Id: I7490976166581abc626f397ad408581ada0ed308
2018-01-25 17:54:50 -08:00
Elliott Hughes
df9a489b2b Add more const-correct C++ overloads.
libc++ actually provides overloads for all the standard C library stuff,
so we just need to handle the POSIX and GNU extensions, of which there
are just two more: memrchr and strcasestr.

Bug: http://b/22768375
Test: builds
Change-Id: Ie9ed1fbcc794e14a0c9bba13b5307ad677949613
2017-08-23 14:34:40 -07:00
George Burgess IV
95bd4884b5 libc fortify: error on realpath(NULL, foo)
I've half a mind to make this a warning instead, since this sort of
call isn't UB. That said:
- if the user really wants this (I can't imagine why they would), they
  can just put NULL in a non-const variable,
- we're slowly moving to -Werror ~everywhere anyway, and
- it's presumably easier to change this from an error to a warning than
  the other way around

Bug: 12231437
Test: m checkbuild on bullhead internal master. No new
CtsBionicTestCases failures.

Change-Id: Ie8bf5a3455f663686fda4a7450fb35d147fa745e
2017-08-14 14:48:55 -07:00
Elliott Hughes
b115aefbb3 With O_TMPFILE, open(2) takes a mode argument.
Strictly, the mode isn't really meaningful unless you supply O_EXCL,
but the kernel will take it and fstat will return it even if you
never give the file a name.

Also warn for O_TMPFILE without a mode at compile time where possible.

Bug: N/A
Test: ran tests
Change-Id: I729b6d6e6190676fd017a1190b6200bf9abdbfd8
2017-08-04 10:20:18 -07:00
Treehugger Robot
cc3639b3ee Merge "libc fortify: warn on open() with useless mode bits" 2017-08-04 07:32:27 +00:00
George Burgess IV
4e37d53f29 libc fortify: warn on open() with useless mode bits
Bug: 64132680
Test: checkbuild on bullhead internal master; no new CtsBionicTestCases
fail.

Change-Id: I98762bbee0ecdd7b4ffc21135992f2eed320317f
2017-08-03 22:04:19 -07:00
Treehugger Robot
b1b125ead1 Merge "libc fortify: make string.h use diagnose_if" 2017-08-03 03:18:44 +00:00
Treehugger Robot
5ac5a7c083 Merge "libc fortify: make fcntl.h use diagnose_if" 2017-08-02 08:06:20 +00:00
Treehugger Robot
a821283d22 Merge "libc fortify: make socket.h and stdlib.h use diagnose_if" 2017-08-02 07:37:46 +00:00
Treehugger Robot
6e0de9da49 Merge "libc fortify: make stdio.h use diagnose_if" 2017-08-02 07:27:23 +00:00
Treehugger Robot
cff14d9e3e Merge "libc fortify: make poll.h and stat.h use diagnose_if" 2017-08-02 06:44:41 +00:00
George Burgess IV
b6300463a8 libc fortify: make string.h use diagnose_if
This also has a handful of style fixups, to make this file more
consistent. And removes __bionic_zero_size_is_okay_t, since there's a
better workaround available.

Bug: 12231437
Test: m checkbuild on bionic internal master; CtsBionicTestCases show
no new failures.

Change-Id: I75a020630dbab0ce828563502900cba14ae992d1
2017-08-01 22:41:11 -07:00
George Burgess IV
54f5d8331f libc fortify: make socket.h and stdlib.h use diagnose_if
Since realpath no longer needs to be overloaded, we can restore the
upstream source to purity. We'll be able to do this with most of the
other functions when we pull a newer clang in.

Bug: 12231437
Test: m checkbuild on bionic internal master; CtsBionicTestCases show
no new failures.

Change-Id: I484221bba0b291273fece23d2be2f5f9fd713d2c
2017-08-01 22:32:15 -07:00
George Burgess IV
23dbf82132 libc fortify: make stdio.h use diagnose_if
This also throws in some reformatting to match the GCC FORTIFY style in
this file (e.g. 4 space standard indent).

Per b/36984245, some function protos are unwrapped beyond 100 chars, as
well.

Bug: 12231437
Test: m checkbuild on bionic internal master; CtsBionicTestCases show
no new failures.

Change-Id: I6dce60ec14d5cd703d9e724699634a197ca11023
2017-08-01 22:02:31 -07:00
George Burgess IV
52dde5fb1c libc fortify: make poll.h and stat.h use diagnose_if
Bug: 12231437
Test: m checkbuild on bionic internal master; CtsBionicTestCases show
no new failures.

Change-Id: I7dbdeca12c46d1ee3804f13546b11c3f3b3a7596
2017-08-01 21:32:12 -07:00