[PATCH] Support/Unix: use lambda function to fix building errors with clang

Hans Wennborg via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 30 02:33:10 PDT 2018


Merged to 7.0 in r341044.

On Mon, Aug 27, 2018 at 5:56 PM, Hans Wennborg <hans at chromium.org> wrote:
> I think I see what's going on now.
>
> This is because of Bionic's _FORTIFY_SOURCE support, where they
> provide overloaded declarations of open():
> https://github.com/aosp-mirror/platform_bionic/blob/b7b2884a2130c32b3ca7228891a0a9aa934a9be7/libc/include/bits/fortify/fcntl.h#L48
>
> That will certainly cause the Clang error you're seeing, regardless of
> Clang version I think.
>
> I've committed r340751 based on Chih-Wei's patch, and will merge to
> 7.0 after it's been in trunk for a little while.
>
> Thanks,
> Hans
>
>
>
> On Mon, Aug 27, 2018 at 3:28 PM, Mauro Rossi <issor.oruam at gmail.com> wrote:
>> Hi Hans,
>>
>> Sorry, I support android-x86 project my skills as programmer are somehow limited
>> For more technical items, please check with Chih-Wei, who made the patch.
>>
>> The patch commit message shows the error happening.
>>
>> If it can help,  my github branch [0], which is building the only
>> commit requiring changes in the llvm codebase is the patch sumbitted
>> to llvm-commit (commit [1])
>> plus some minor reverts due to warnings treated as errors, all the
>> rest is building rules/sources lists.
>> BTW I am working on soong  building rules, which are more easy to maintain.
>>
>> I would understand if you consider Android a non supported system and
>> we can live without having the patch in the llvm release_70 codebase
>> (it is one commit and the important thing is we can build, of course).
>>
>> I am available for providing further build logs, even if I do not see
>> too much the point, but I may need instructions to do so.
>> Mauro
>>
>> [0] https://github.com/maurossi/llvm/commits/oreo-x86_llvm70_release_70
>> [1] https://github.com/maurossi/llvm/commit/e15165b1c28562d50c36b592347b55afb101308c
>> Il giorno lun 27 ago 2018 alle ore 14:09 Hans Wennborg
>> <hans at chromium.org> ha scritto:
>>>
>>> I tried building Path.cpp using Clang built from r300080, but was
>>> unable to reproduce the problem.
>>>
>>> I suppose the declaration of ::open must be different than on my system.
>>>
>>> Can you send a preprocessed source file and compiler invocation that
>>> shows the error?
>>>
>>> On Mon, Aug 27, 2018 at 12:25 PM, Mauro Rossi <issor.oruam at gmail.com> wrote:
>>> > Re-added Pavel, Chih-We and llvm-commits ML,
>>> > which had erronously omitted in my previous message
>>> >
>>> > Mauro
>>> >
>>> > Il giorno lun 27 ago 2018 alle ore 12:22 Mauro Rossi
>>> > <issor.oruam at gmail.com> ha scritto:
>>> >>
>>> >> Hi Hans,
>>> >> thanks for your response
>>> >>
>>> >> Il giorno lun 27 ago 2018 alle ore 10:35 Hans Wennborg
>>> >> <hans at chromium.org> ha scritto:
>>> >> >
>>> >> > On Sat, Aug 25, 2018 at 1:22 PM, Mauro Rossi <issor.oruam at gmail.com> wrote:
>>> >> > > Hi Pavel,
>>> >> > >
>>> >> > > Il giorno mar 17 lug 2018 alle ore 08:36 Mauro Rossi
>>> >> > > <issor.oruam at gmail.com> ha scritto:
>>> >> > >>
>>> >> > >> Hi Pavel,
>>> >> > >>
>>> >> > >> Il giorno lun 16 lug 2018 alle ore 19:14 Pavel Labath <labath at google.com> ha scritto:
>>> >> > >>>
>>> >> > >>> I think all/most systems have open(2) defined as a variadic (...)
>>> >> > >>> function, so that can't be the only reason why this is failing for
>>> >> > >>> you. There must be something more which is specific to your platform.
>>> >> > >>> Nevertheless, the workaround is not bad, so there's no harm in
>>> >> > >>>
>>> >> > >>> changing this. Some people would even prefer the lambda based syntax.
>>> >> > >>
>>> >> > >>
>>> >> > >> We are using basically the same Build System/compilers of AOSP,
>>> >> > >> it seam that just using clang can produce the error because of behavior different from gcc.
>>> >> > >> In some comments in the web behaviors of gcc and clang were described as different.
>>> >> > >>
>>> >> > >> If you are also using clang, are you using the clang version with AOSP (8.1.0_r33 at the time of reporting)
>>> >> > >> I propose to check in order to identify root cause, then evaluate how to proceed.
>>> >> > >>
>>> >> > >> Mauro
>>> >> > >
>>> >> > > The building error is happening using default AOSP 8.1.0_r41 clang tool chain
>>> >> >
>>> >> > Can you print the "clang --version" output of that toolchain? If we
>>> >> > fix this, it would be good to put a comment about what compiler was
>>> >> > having a problem.
>>> >>
>>> >> As seen from build log this is the clang version (also used by AOSP,
>>> >> if I'm not wrong):
>>> >>
>>> >> maurossi at ANDROIDDEV:~/oreo-x86_kernel$
>>> >> prebuilts/clang/host/linux-x86/clang-4053586/bin/clang --version
>>> >> Android clang version 5.0.300080  (based on LLVM 5.0.300080)
>>> >> Target: x86_64-unknown-linux
>>> >> Thread model: posix
>>> >> InstalledDir: /home/maurossi/oreo-x86_kernel/prebuilts/clang/host/linux-x86/clang-4053586/bin
>>> >>
>>> >>
>>> >> >
>>> >> > > If you don't see drawbacks, would it be possible to queue the patch
>>> >> > > that solves the building problem
>>> >> > > as candidate for llvm 7.0rc3?
>>> >> > > This would prevent building issues for AOSP current and future Android releases
>>> >> > >
>>> >> > > After your feedback, I am adding Hans Wennborg to evaluate if the
>>> >> > > patch is an acceptable candidate for llvm 7.0rc3
>>> >> >
>>> >> > I'm OK with this, but the patch would have to land soon.
>>> >>
>>> >> The patch was sent on llvm-commits mailing list on July, 8th 2018,
>>> >> it can be applied in current release_70 branch as cherry-pick is working for me:
>>> >>
>>> >> http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20180702/567254.html
>>> >>
>>> >> Could you please modify the final commit message with clang version
>>> >> information provided?
>>> >> Thanks
>>> >>
>>> >> Kind regards
>>> >> Mauro
>>> >>
>>> >>
>>> >>
>>> >>
>>> >> >
>>> >> > Thanks,
>>> >> > Hans


More information about the llvm-commits mailing list