erichkeane created this revision.
erichkeane added reviewers: efriedma, eli.friedman, compnerd, rsmith.
erichkeane added a project: clang.
As reported here: https://bugs.llvm.org/show_bug.cgi?id=37033
Any usage of a builtin function that uses a va_list by reference
will cause an assertion when red
efriedma added a comment.
Rather than adding weird hacks to merging, can we just reject any attempt to
redeclare a builtin?
Repository:
rC Clang
https://reviews.llvm.org/D45383
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists
erichkeane added a comment.
In https://reviews.llvm.org/D45383#1060104, @efriedma wrote:
> Rather than adding weird hacks to merging, can we just reject any attempt to
> redeclare a builtin?
I don't believe we can. I think things often re-declare them, particularly
when it comes to va_end.
erichkeane added a comment.
One big issue with just disallowing redecls of builtins is we have is that a
number of these are actually expected to be in the system headers/libraries
(such as vprintf). A ton of the math.h functions are also builtins.
Repository:
rC Clang
https://reviews.llvm
efriedma added a comment.
By "builtins" I meant specifically the stuff that that user code isn't allowed
to declare, like __builtin_*, since they often have weird/incomplete signatures.
Repository:
rC Clang
https://reviews.llvm.org/D45383
___
cf
erichkeane added a comment.
In https://reviews.llvm.org/D45383#1060275, @efriedma wrote:
> By "builtins" I meant specifically the stuff that that user code isn't
> allowed to declare, like __builtin_*, since they often have weird/incomplete
> signatures.
Unfortunately, the problem shows in vp
efriedma added a comment.
How could this patch possibly affect vprintf?
Repository:
rC Clang
https://reviews.llvm.org/D45383
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
erichkeane added a comment.
In https://reviews.llvm.org/D45383#1060354, @efriedma wrote:
> How could this patch possibly affect vprintf?
vprintf is defined in the builtins.def file, and other than our messages
considering it a 'library function', it is otherwise identical.
Repository:
rC C
efriedma added a comment.
Identical to what? `__builtin_va_start` and `__builtin_va_end` specifically
are weird because they're builtins which have a signature which can't be
expressed in C. vprintf doesn't have that problem.
Builtins.def makes the relevant distinction already: a "BUILTIN" is
erichkeane added a comment.
In https://reviews.llvm.org/D45383#1061780, @efriedma wrote:
> Identical to what? `__builtin_va_start` and `__builtin_va_end` specifically
> are weird because they're builtins which have a signature which can't be
> expressed in C. vprintf doesn't have that problem
efriedma added a comment.
> vprintf is handled using the exact same code-paths. SO, it'll have its 2nd
> parameter created with type 'char*&'
vprintf is defined in the C standard as "int vprintf(const char *format,
va_list arg);"; on Windows, that's equivalent to "int vprintf(const char
*forma
erichkeane added a comment.
In https://reviews.llvm.org/D45383#1061797, @efriedma wrote:
> > vprintf is handled using the exact same code-paths. SO, it'll have its 2nd
> > parameter created with type 'char*&'
>
> vprintf is defined in the C standard as "int vprintf(const char *format,
> va_list
efriedma added a comment.
> I don't see redeclaring the latter 3 as being disallowed,
It is in fact disallowed by the standard, in the section describing
va_start/va_arg/va_end/va_copy: "If a macro definition is suppressed in order
to access an actual function, or a program defines an external
erichkeane added a comment.
In https://reviews.llvm.org/D45383#1062014, @efriedma wrote:
> > I don't see redeclaring the latter 3 as being disallowed,
>
> It is in fact disallowed by the standard, in the section describing
> va_start/va_arg/va_end/va_copy: "If a macro definition is suppressed in
efriedma added a comment.
> I believe it is permissible to implement va_copy and va_end as a function,
> isn't it?
I guess you could, in theory, but LLVM doesn't support any targets which do
that.
Repository:
rC Clang
https://reviews.llvm.org/D45383
_
erichkeane added a comment.
In https://reviews.llvm.org/D45383#1062078, @efriedma wrote:
> > I believe it is permissible to implement va_copy and va_end as a function,
> > isn't it?
>
> I guess you could, in theory, but LLVM doesn't support any targets which do
> that.
Ok, fair enough. So is
efriedma added a comment.
Basically, yes, although I was thinking you could be a bit more aggressive. At
least, anything marked "t" for custom typechecking is probably not something
which should be redeclared.
Repository:
rC Clang
https://reviews.llvm.org/D45383
erichkeane added a comment.
In https://reviews.llvm.org/D45383#1062099, @efriedma wrote:
> Basically, yes, although I was thinking you could be a bit more aggressive.
> At least, anything marked "t" for custom typechecking is probably not
> something which should be redeclared.
Ok, thank you
18 matches
Mail list logo