New issue
Advanced search Search tips

Issue 917949 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 4
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug
Build-Toolchain

Blocking:
issue 916403



Sign in to add a comment

Got compile error of Gentoo package if there is strings.h at local directory.

Project Member Reported by menghuan@chromium.org, Dec 27

Issue description

In  bug 916403 , I am trying to upgrade to a new version of package.

In that package, there is a strings.h in the local directory. The default compiler option of gentoo contains "-I." and "-O2", so that including string.h will result in including strings.h in /usr/include/string.h:431.

A simple way to reproduce this is that 
1. enter chroot
2. creating a directory with an empty file called "strings.h" and write a code include <string.h>
in the same directory.
  echo -e "#include<string.h>\nint main(){}" > a.c && touch strings.h

3. compile the code with (-I. and -O2 are needed.)
  x86_64-cros-linux-gnu-clang -I.  -O2 -c a.c

Then got errors:
In file included from a.c:1:
In file included from ../../usr/x86_64-cros-linux-gnu/usr/include/string.h:494:
../../usr/x86_64-cros-linux-gnu/usr/include/bits/string_fortified.h:116:6: error: use of undeclared identifier '__bos'
     __warn_if_src_too_large (__dest, __src)
     ^
../../usr/x86_64-cros-linux-gnu/usr/include/bits/string_fortified.h:40:41: note: expanded from macro '__warn_if_src_too_large'
  __clang_warning_if (__size_too_small (__bos, dest, __builtin_strlen (src) + 1), \
                                        ^
../../usr/x86_64-cros-linux-gnu/usr/include/bits/string_fortified.h:136:6: error: use of undeclared identifier '__bos'
     __warn_if_dest_too_small (__dest, __len)
     ^
../../usr/x86_64-cros-linux-gnu/usr/include/bits/string_fortified.h:31:41: note: expanded from macro '__warn_if_dest_too_small'
  __clang_warning_if (__size_too_small (__bos, dest, len), \
                                        ^
../../usr/x86_64-cros-linux-gnu/usr/include/bits/string_fortified.h:150:6: error: use of undeclared identifier '__bos'
     __warn_if_dest_too_small (__dest, __n)
     ^
../../usr/x86_64-cros-linux-gnu/usr/include/bits/string_fortified.h:31:41: note: expanded from macro '__warn_if_dest_too_small'
  __clang_warning_if (__size_too_small (__bos, dest, len), \
                                        ^
../../usr/x86_64-cros-linux-gnu/usr/include/bits/string_fortified.h:160:6: error: use of undeclared identifier '__bos'
     __warn_if_src_too_large (__dest, __src)
     ^
../../usr/x86_64-cros-linux-gnu/usr/include/bits/string_fortified.h:40:41: note: expanded from macro '__warn_if_src_too_large'
  __clang_warning_if (__size_too_small (__bos, dest, __builtin_strlen (src) + 1), \
                                        ^
../../usr/x86_64-cros-linux-gnu/usr/include/bits/string_fortified.h:168:6: error: use of undeclared identifier '__bos'
     __warn_if_src_too_large (__dest, __src)
     ^
../../usr/x86_64-cros-linux-gnu/usr/include/bits/string_fortified.h:40:41: note: expanded from macro '__warn_if_src_too_large'
  __clang_warning_if (__size_too_small (__bos, dest, __builtin_strlen (src) + 1), \
                                        ^
5 errors generated.


 
i think it's due to the patch we're carrying for clang/fortify in glibc.  it looks like __bos is being used in a code path that doesn't include sys/cdefs.h.
since opensc is only used for testing, i don't think this is a P1
Labels: -Pri-1 Pri-2
Owner: g...@chromium.org
Status: Assigned (was: Untriaged)
assigning to gbiv. He will look at it next week. Let me know if you need it that is not ok.


Description: Show this description
Thanks for this!

Looks like string_fortified.h now has an #include of strings_fortified.h in the middle of it. The FORTIFY patch has them both `#define`ing and `#undef`ing __size_too_small. Since macro #defines/#undefs don't nest, strings_fortified.h takes __size_too_small away from string_fortified.h.

To explain why this hasn't been an issue in the past:
- this bug only happens when string_fortified.h includes strings_fortified.h for the first time.
- glibc's string.h includes strings.h before including string_fortified.h.
- glibc's strings.h includes strings_fortified.h.
- with an empty strings.h, glibc's attempt to #include strings.h pulls nothing in, so string_fortified's #include of strings_fortified will actually do things, instead of being header guarded into nothing.

Patch coming soon.
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/3c82d1289f67f6775d9df151f6df978ffa48b6fe

commit 3c82d1289f67f6775d9df151f6df978ffa48b6fe
Author: George Burgess IV <gbiv@google.com>
Date: Fri Jan 04 10:09:05 2019

glibc: Use different macro names in string.h/strings.h

If glibc doesn't include strings_fortified.h before including
string_fortified.h (which it tries to, but may be unsuccessful if
there's a ./strings.h file), strings_fortified.h will be included by
string_fortified.h. The former and latter both #define and #undef a
few identically-named macros, so strings_fortified.h ends up taking away
macros that string_fortified.h depends on.

'Namespacing' the macros in strings_fortified.h is the simplest way to
avoid this.

BUG= chromium:917949 
TEST=touch strings.h &&
	echo '#include <string.h>' |
		x86_64-cros-linux-gnu-clang -I. -O2 -x c -
     Also built falco from scratch.

Change-Id: If74903f2af92264a2b51782e22bd7de8cd6284a4
Reviewed-on: https://chromium-review.googlesource.com/1393435
Commit-Ready: George Burgess <gbiv@chromium.org>
Tested-by: George Burgess <gbiv@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/3c82d1289f67f6775d9df151f6df978ffa48b6fe/sys-libs/glibc/files/local/glibc-2.27-clang-fortify.patch
[rename] https://crrev.com/3c82d1289f67f6775d9df151f6df978ffa48b6fe/sys-libs/glibc/glibc-2.27-r8.ebuild

Status: Fixed (was: Assigned)
With a freshly-synced chroot:

$ touch strings.h && echo -e '#include <string.h>\nint main() {}' | x86_64-cros-linux-gnu-clang++ -x c++ -O2 -I. - -D_FORTIFY_SOURCE=2 && echo yay
yay
$

So this appears to be fixed. Please let me know if I missed anything
I still got the same problem. I already do repo sync and cros_sdk --delete to cleanup the whole chroot.

(cr) menghuan@cerberus /tmp/test $ touch strings.h && echo -e '#include <string.h>\nint main() {}' | x86_64-cros-linux-gnu-clang++ -x c++ -O2 -I. - -D_FORTIFY_SOURCE=2 && echo yay
In file included from <stdin>:1:
In file included from /usr/bin/../include/c++/v1/string.h:61:
In file included from /usr/x86_64-cros-linux-gnu/usr/include/string.h:494:
/usr/x86_64-cros-linux-gnu/usr/include/bits/string_fortified.h:116:6: error: use of undeclared identifier '__bos'
     __warn_if_src_too_large (__dest, __src)
     ^
/usr/x86_64-cros-linux-gnu/usr/include/bits/string_fortified.h:40:41: note: expanded from macro '__warn_if_src_too_large'
  __clang_warning_if (__size_too_small (__bos, dest, __builtin_strlen (src) + 1), \
                                        ^
/usr/x86_64-cros-linux-gnu/usr/include/bits/string_fortified.h:125:6: error: use of undeclared identifier '__bos'
     __warn_if_src_too_large (__dest, __src)
     ^
/usr/x86_64-cros-linux-gnu/usr/include/bits/string_fortified.h:40:41: note: expanded from macro '__warn_if_src_too_large'
  __clang_warning_if (__size_too_small (__bos, dest, __builtin_strlen (src) + 1), \
                                        ^
/usr/x86_64-cros-linux-gnu/usr/include/bits/string_fortified.h:136:6: error: use of undeclared identifier '__bos'
     __warn_if_dest_too_small (__dest, __len)
     ^
/usr/x86_64-cros-linux-gnu/usr/include/bits/string_fortified.h:31:41: note: expanded from macro '__warn_if_dest_too_small'
  __clang_warning_if (__size_too_small (__bos, dest, len), \
                                        ^
/usr/x86_64-cros-linux-gnu/usr/include/bits/string_fortified.h:150:6: error: use of undeclared identifier '__bos'
     __warn_if_dest_too_small (__dest, __n)
     ^
/usr/x86_64-cros-linux-gnu/usr/include/bits/string_fortified.h:31:41: note: expanded from macro '__warn_if_dest_too_small'
  __clang_warning_if (__size_too_small (__bos, dest, len), \
                                        ^
/usr/x86_64-cros-linux-gnu/usr/include/bits/string_fortified.h:160:6: error: use of undeclared identifier '__bos'
     __warn_if_src_too_large (__dest, __src)
     ^
/usr/x86_64-cros-linux-gnu/usr/include/bits/string_fortified.h:40:41: note: expanded from macro '__warn_if_src_too_large'
  __clang_warning_if (__size_too_small (__bos, dest, __builtin_strlen (src) + 1), \
                                        ^
/usr/x86_64-cros-linux-gnu/usr/include/bits/string_fortified.h:168:6: error: use of undeclared identifier '__bos'
     __warn_if_src_too_large (__dest, __src)
     ^
/usr/x86_64-cros-linux-gnu/usr/include/bits/string_fortified.h:40:41: note: expanded from macro '__warn_if_src_too_large'
  __clang_warning_if (__size_too_small (__bos, dest, __builtin_strlen (src) + 1), \
                                        ^
6 errors generated.
--- EOM ---


It looks like the /usr/include/bits/strings_fortified.h applied your patch, but /usr/x86_64-cros-linux-gnu/usr/include/bits/strings_fortified.h didn't.

(cr) menghuan@cerberus /tmp/test $ diff /usr/include/bits/strings_fortified.h /usr/x86_64-cros-linux-gnu/usr/include/bits/strings_fortified.h
22c22
< #define __strings_warn_len_too_large \
---
> #define __warn_len_too_large \
25c25
< #define __strings_size_too_small(dest, len) \
---
> #define __size_too_small(dest, len) \
31,32c31,32
<      __clang_warning_if (__strings_size_too_small (__dest, __len),
<                        __strings_warn_len_too_large)
---
>      __clang_warning_if (__size_too_small (__dest, __len),
>                        __warn_len_too_large)
39,40c39,40
<      __clang_warning_if (__strings_size_too_small (__dest, __len),
<                        __strings_warn_len_too_large)
---
>      __clang_warning_if (__size_too_small (__dest, __len),
>                        __warn_len_too_large)
46,47c46,47
< #undef __strings_size_too_small
< #undef __strings_warn_len_too_large
---
> #undef __size_too_small
> #undef __warn_len_too_large
menghuan@ does it work if you run ./update_chroot --nousepkg first?

( The way Chrome OS builders are set up currently, it can take glibc or any other toolchain upgrade to take around 20-40 hrs or more post-commit to be effective.)
I suspect that this is due to a stale crossdev package in your chroot. I don't know our story for how crossdev packages get rebuilt and redeployed on updates, but I'd imagine you can fix this locally by reemerging them all:

$ sudo emerge -j16 $(equery list '*' | grep '/glibc-' | sed -E 's/-2\.[0-9][0-9]-r[0-9]+$//')

Let me ping a few people so I can get a better understanding of how that all works.
Or do what Manoj says. He knows these processes way better than I do ;)
./update_chroot --nousepkg works. Thanks.

It seems the patch is also updated. I tried repo sync again and it looks fine.

Sign in to add a comment