<!DOCTYPE html><html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<div class="moz-cite-prefix">On 12/1/24 2:55 AM, Julian Waters
wrote:<br>
</div>
<blockquote type="cite" cite="mid:CAP2b4GP-O3MPXJgCbeQYtb6mjhqvr5BxFwJB6ZaDf=27NSaHkg@mail.gmail.com">
<pre wrap="" class="moz-quote-pre">This seems to change the forbidden interface, which by itself is ok,
but I have some questions</pre>
</blockquote>
<p><span style="white-space: pre-wrap">Changing the syntax also doesn't bother me. I've always expected that if the
underlying mechanism got changed then the usage syntax might need to change.
There aren't that many uses (there really shouldn't be lots of "allows"). I
think there are currently ~35 allows?
</span></p>
<blockquote type="cite" cite="mid:CAP2b4GP-O3MPXJgCbeQYtb6mjhqvr5BxFwJB6ZaDf=27NSaHkg@mail.gmail.com">
<pre wrap="" class="moz-quote-pre">1. Changing from signature to a name might make the forbidden function
less specific, as information on exactly which function is forbidden
is lost. Would this create problems in the event that another function
with the same name but different signature which is not forbidden is
called?</pre>
</blockquote>
The purpose of this mechanism is to prevent the (non-casual) use of
certain C<br>
library functions. C doesn't support overloading. So different
signatures is<br>
not an issue.<br>
<br>
It also doesn't matter, since we're manipulating name lookup. If
there were<br>
two overloads of malloc (or frob, in my example), the forbidding
still blocks<br>
access and the permitting just has to call with the proper
arguments. That's<br>
also a reason to *not* base the forbidding declaration on the
function being<br>
blocked. It can be anything, and making it a deleted and deprecated
function<br>
isn't useful. Mine works fine even if there are multiple overloaded<br>
declarations of the global frob function.<br>
<br>
<blockquote type="cite" cite="mid:CAP2b4GP-O3MPXJgCbeQYtb6mjhqvr5BxFwJB6ZaDf=27NSaHkg@mail.gmail.com">
<pre wrap="" class="moz-quote-pre">
2. ALLOW_C_FUNCTION used to take a statement, changing it to a call
expression would mean every use site would have to change as well.
Many uses of the macro also use qualified names as well.</pre>
</blockquote>
Again, the syntax change doesn't bother me.<br>
<br>
The statement-based syntax for the old ALLOW_C_FUNCTION is a matter
of history<br>
and the mechanism used to implement the feature. The implementation
involved<br>
setting up a surrounding pragma context, which required a
statement. At that<br>
time I had not yet realized that lambdas provide some of the
functionality<br>
needed to be expression-based while being able to establish such a
surrounding<br>
pragma context. (gcc supports statements and declarations in
expressions as<br>
an extension. lambda provides a portable mechanism for doing
that.) Had I<br>
realized that at the time, I would have made ALLOW_C_FUNCTION
expression-based.<br>
<br>
<blockquote type="cite" cite="mid:CAP2b4GP-O3MPXJgCbeQYtb6mjhqvr5BxFwJB6ZaDf=27NSaHkg@mail.gmail.com">
<pre wrap="" class="moz-quote-pre"> There's also
the VC specific warning disable in compilerWarnings, which I'm not
sure why it was there in the first place when FORBID_C_FUNCTION does
nothing on VC: <a class="moz-txt-link-freetext" href="https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/28ae281b42cd00f471e275db544a5d23a42df59c/src/hotspot/share/utilities/compilerWarnings_visCPP.hpp*L64__;Iw!!ACWV5N9M2RV99hQ!L0gUwZ8rkVwClErEUKrf4NKjn2-hjhuJu4_a1D6pE2GMOOc8KLuxs31FpxcObtQDkzbcsTmx25hpInegCuhgUM3C$">https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/28ae281b42cd00f471e275db544a5d23a42df59c/src/hotspot/share/utilities/compilerWarnings_visCPP.hpp*L64__;Iw!!ACWV5N9M2RV99hQ!L0gUwZ8rkVwClErEUKrf4NKjn2-hjhuJu4_a1D6pE2GMOOc8KLuxs31FpxcObtQDkzbcsTmx25hpInegCuhgUM3C$</a>
Now that ALLOW_C_FUNCTION takes an expression, I don't think that same
warning disable can be done, would that cause issues for
ALLOW_C_FUNCTION when VC is the compiler?</pre>
</blockquote>
The comment about the Windows definition of ALLOW_C_FUNCTIONS
discusses why it<br>
exists. At the time, it seemed like it might be useful for
suppressing<br>
deprecation warnings from other sources, such as the "security"
warnings and<br>
from _CRT_DECLARE_NONSTDC_NAMES. But we ultimately decided to just
globally<br>
use the warning suppression macros for those, so didn't need to use
the ALLOW<br>
macro for that purpose. So the current definition doesn't really
serve any<br>
purpose.<br>
<br>
<blockquote type="cite" cite="mid:CAP2b4GP-O3MPXJgCbeQYtb6mjhqvr5BxFwJB6ZaDf=27NSaHkg@mail.gmail.com">
<pre wrap="" class="moz-quote-pre">
3. None of the big 3 compilers inline lambdas by default, only when
optimizations are enabled. In practice this means slowdebug HotSpot
will have the wrapper lambda in the final assembly instead of a direct
call to the C library code. I don't know if this issue is a deal
breaker</pre>
</blockquote>
Regarding non-inlining of lambdas in unoptimized code, I don't
really care all<br>
that much. If it's *really* an issue,
`__attribute__((always_inline))` or<br>
`[[gnu::always_inline]]` can help. But the approach of requiring the
use of a<br>
specially qualified name instead of a context macro does have some
appeal. But<br>
I think I'd want a less convenient name. The point of this mechanism
is to<br>
make it intentionally inconvenient and obvious that the blockade is
being<br>
bypassed, because it shouldn't be done casually or "quietly".<br>
<blockquote type="cite" cite="mid:CAP2b4GP-O3MPXJgCbeQYtb6mjhqvr5BxFwJB6ZaDf=27NSaHkg@mail.gmail.com">
<pre wrap="" class="moz-quote-pre">
In any case I tried again after seeing the new example, but using
separate macros to handle the permitted namespace and the actual
forbidding of the function itself. I used the older style signature
based forbidding rather than the new name based solution (Due to
concerns over #1), but I included a toggle to switch between name
based and signature based variants anyway, so a comparison between the
2 can easily be made. Rather than ALLOW_C_FUNCTION this example
directly does permitted::symbol, the meaning of which is also rather
clear
<a class="moz-txt-link-freetext" href="https://urldefense.com/v3/__https://godbolt.org/z/hdhrh5eev__;!!ACWV5N9M2RV99hQ!L0gUwZ8rkVwClErEUKrf4NKjn2-hjhuJu4_a1D6pE2GMOOc8KLuxs31FpxcObtQDkzbcsTmx25hpInegCsNpF1Dt$">https://urldefense.com/v3/__https://godbolt.org/z/hdhrh5eev__;!!ACWV5N9M2RV99hQ!L0gUwZ8rkVwClErEUKrf4NKjn2-hjhuJu4_a1D6pE2GMOOc8KLuxs31FpxcObtQDkzbcsTmx25hpInegCsNpF1Dt$</a>
</pre>
</blockquote>
I think separate macros for registering the bypass and registering
the<br>
forbidding is a mistake, and just has the potential for accidental
misuse.<br>
The bypass *must* be registered first, and there's no point to it
without the<br>
forbidding, so one macro is better.<br>
<p>I've also dropped the "alternative" string. It doesn't do
anything useful in<br>
the code. Depending on which compiler is used, it may or may not
appear in the<br>
error message. I think just a comment at the point of forbidding
would<br>
suffice, and can be more extensive than one would want in a
string, if/when<br>
that's warranted. Anyone who is encountering a failure clearly has
the source<br>
code available, and the error messages from all three compilers
seems to<br>
provide information about where in the source to look.<br>
<br>
How about this:<br>
<br>
void* frob(unsigned);<br>
void* frob(unsigned, unsigned);<br>
<br>
// name must be unqualified.<br>
#define FORBID_C_FUNCTION(name) \<br>
namespace permit_forbidden_functions { using ::name; } \<br>
inline namespace forbidden_functions { const int name = 0; } \<br>
/* */<br>
<br>
void* permit_frob(unsigned x) {<br>
return permit_forbidden_functions::frob(x);<br>
}<br>
<br>
void* permit_frob(unsigned x, unsigned y) {<br>
return permit_forbidden_functions::frob(x, y);<br>
}<br>
<br>
// demonstrate error for unqualified call.<br>
#if 0 // change to 1 to trigger error <br>
void* bad_frob1(unsigned x) {<br>
return frob(x);<br>
}<br>
#endif<br>
<br>
// demonstrate error for qualified call.<br>
#if 0 // change to 1 to trigger error <br>
void* bad_frob2(unsigned x) {<br>
return ::frob(x);<br>
}<br>
#endif<br>
<br>
</p>
</body>
</html>