[cfe-dev] RFC: do not optimize on basis of __attribute__((nonnull)) in glibc headers
Philip Reames via cfe-dev
cfe-dev at lists.llvm.org
Fri Mar 17 18:36:07 PDT 2017
I'm in support of unblocking the LLVM side optimization changes. I'm
definitely not qualified to speak towards the direction of the clang
changes though. :)
Philip
On 03/16/2017 03:39 PM, Chandler Carruth via cfe-dev wrote:
> Are there blockers to landing https://reviews.llvm.org/D30806 ?
>
> I'm committed to following through with the respective standards
> bodies and anyone else we feel is prudent, but I would somewhat like
> to unblock the optimization work in this area in parallel as I suspect
> that the communication and establishment of a rational plan will
> require a lot of time.
>
> On Tue, Mar 14, 2017 at 2:54 PM Chandler Carruth via cfe-dev
> <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
>
> I'm happy to handle at least the standards side of this; I don't
> really have great connections on the libc front, but happy to help
> folks do that.
>
> On Tue, Mar 14, 2017 at 2:35 PM James Y Knight
> <jyknight at google.com <mailto:jyknight at google.com>> wrote:
>
> Just to reiterate...
>
> I would really like to see us communicate this decision, with
> an appropriately persuasive argument, both to the glibc
> authors and to the standards committees, and treat this as a
> *transitional* hack, which is needed until glibc (and any
> other libc if there are any?) can remove the erroneous nonnull
> declarations from their headers, and everyone has upgraded.
>
> On Thu, Mar 9, 2017 at 11:04 PM, Chandler Carruth
> <chandlerc at google.com <mailto:chandlerc at google.com>> wrote:
>
> I've sent a patch to implement this here:
> https://reviews.llvm.org/D30806
>
> It turns out to be both easier and I hope less
> controversial than I had imagined. Neither Clang nor LLVM
> ever actually got nonnull onto these declarations even
> when they were suitable annotated. We didn't carry that
> annotation across to the declaration. Oops.
>
> So I've fixed that, but *also* disabled it for the libc
> functions that we have library builtin recognition for. We
> can extend this to cover any set of library functions
> folks want, just let me know. The use of it will even be
> correctly controlled by -fno-builtin and friends.
>
> The net result is that this should not regress
> optimizations for *anyone*, and actually enable a bunch of
> optimizations outside of libc functions, while preserving
> safety on those libc functions.
>
> We still have the nullability attributes which can be used
> to annotate more interfaces in a way that will provide
> warnings and static analysis aid, but not optimize on and
> so not run the risk of changing behavior of existing code.
> Those would be good candidates (IMO) for libc++ to use on
> any relevant interfaces which match the criteria outlined
> in this thread: pointers paired with a size should allow
> null+zero.
>
> Hopefully this makes everyone happy, please feel free to
> chime in on the review thread if more discussion is needed.
>
> On Wed, Jan 4, 2017 at 11:24 AM Aaron Ballman via cfe-dev
> <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>>
> wrote:
>
> On Wed, Jan 4, 2017 at 2:01 PM, James Y Knight
> <jyknight at google.com <mailto:jyknight at google.com>> wrote:
> >
> >
> > On Wed, Jan 4, 2017 at 12:54 PM, Hal Finkel
> <hfinkel at anl.gov <mailto:hfinkel at anl.gov>> wrote:
> >>
> >>
> >> On 01/04/2017 11:43 AM, James Y Knight via cfe-dev
> wrote:
> >>
> >> On Wed, Jan 4, 2017 at 11:12 AM, Aaron Ballman via
> cfe-dev
> >> <cfe-dev at lists.llvm.org
> <mailto:cfe-dev at lists.llvm.org>> wrote:
> >>>
> >>> So I would be opposed to ignoring those attributes in
> >>>
> >>> Sema (I think we should still warn when users do
> nonportable things),
> >>> but in favor of not changing the optimizer to
> capitalize on this
> >>> "opportunity."
> >>
> >>
> >> I'd be opposed to ignoring the attributes only in
> some places and not in
> >> others. It should be ignored totally, as if it
> wasn't present on those
> >> functions. Doing anything else sends the wrong
> message -- that libc authors
> >> should continue to use nonnull on these functions
> because they might be
> >> helpful, and won't do anything bad.
> >>
> >>
> >> I think that we have a responsibility to our users
> to continue to warn
> >> (statically, in ubsan, etc.) on non-portable
> behavior, which this is and
> >> will continue to be in practice for at least a
> decade or two, regardless of
> >> the message we'd like to send libc authors. We
> cannot undo history here and
> >> this will be relevant to production systems for at
> least a decade. We can
> >> talk to libc developers directly -- they're a much
> smaller set -- and we can
> >> pursue change at the standards level while still
> providing the most useful
> >> set of tools to our users in the mean time.
> >
> >
> > But, this is an entirely different question.
> >
> > - Should clang warn about non-portable usage of
> passing NULL to memcpy/etc?
> >
> > Sounds like a fine warning to add.
> >
> > - Should that warning be dependent on the libc
> headers having nonnull
> > annotations on these functions, which will be used
> only for warnings, and
> > ignored for semantics, on this given list of
> hardcoded functions?
> >
> > No.
> >
> > Firstly: I'd note that nearly all libc
> implementations don't use these
> > attributes today. In some cases, because they've
> simply not thought about
> > it, but in others because they explicitly decided to
> NOT break their users'
> > code by introducing this problem! Glibc is the
> outlier, here.
> >
> > So: what portability do you want to warn for?
> Portability assuming the same
> > libc, but a different compiler which might fail to
> ignore the nonnull
> > attribute? Or portability to other libc? If the
> latter, depending on the
> > nonnull attribute being present doesn't and can't work.
>
> My preference is for portability for both, but you
> bring up a good
> point about other libc implementations not being
> annotated. So long as
> we retain the ability to tell users their code is not
> portable *and*
> we get rid of the dangerous optimizations, I'm happy.
> I just don't
> want to lose the diagnostics because of the optimizations.
>
> ~Aaron
>
> > Secondly: if we already have a hardcoded list of
> functions to special case,
> > that could just as well be used to generate a
> nonportable-stringfunc-null
> > warning, as well.
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20170317/b70131c8/attachment.html>
More information about the cfe-dev
mailing list