Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Xcode 14.1RC command line tools bootstrap fail #98

Closed
iains opened this issue Oct 18, 2022 · 24 comments
Closed

Xcode 14.1RC command line tools bootstrap fail #98

iains opened this issue Oct 18, 2022 · 24 comments

Comments

@iains
Copy link
Owner

iains commented Oct 18, 2022

I tried the RC on my latest master ...

/src-local/gcc-master/libcpp/lex.cc:4623:15: error: ‘int sprintf(char*, const char*, ...)’ is deprecated: This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror=deprecated-declarations]

Anyone else (@fxcoudert ?) have experience with this RC on other branches?

@iains
Copy link
Owner Author

iains commented Oct 18, 2022

I cannot see any statement in either the C23 or the Posix 7 documentation that says that sprintf is deprecated, so this seems to be a bug.

@iains
Copy link
Owner Author

iains commented Oct 18, 2022

( FAOD, I agree completely with the comment that this function has security concerns .. but 'highly recommended' is not the same thing as saying that the function is deprecated )

This seems to be a place that the compiler should warn (for -Wextra or similar) not that should make functional builds fail.

@iains
Copy link
Owner Author

iains commented Oct 18, 2022

I bootstrapped with -Wno-deprecated-declarations but there seem to be a large number of regressions in the testsuite too .. (not analysed yet) - would be interested in any one else's experience with the 14.1RC.

edit: many (maybe most) of the objective c fails come from unguarded blocks use in objc/runtime.h.

@fxcoudert
Copy link
Contributor

Just installed macOS 13.0 RC, and CLT 14.1 RC. I have not yet done a build, but I am not seeing an issue at the moment with a simple test case:

$ cat a.c                                           
#include <stdio.h>

int main (void) {
  char buf[1024];
  sprintf(buf, "%d", 42);
  fputs(buf, stderr);
  return 0;
}
$ clang a.c -Werror=deprecated-declarations -W -Wall
$

Is your issue coming from the headers, from GCC itself, or from the CLT tools?

@fxcoudert
Copy link
Contributor

I see the message in the headers, though, for the macOS 13 SDK:

__swift_unavailable("Use snprintf instead.")
#if !defined(_POSIX_C_SOURCE)
__deprecated_msg("This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead.")
#endif
int      sprintf(char * __restrict, const char * __restrict, ...) __printflike(2, 3);

Apparently it was already there in prior betas: dotnet/runtime#71561

@iains
Copy link
Owner Author

iains commented Oct 19, 2022

I am on 12.6 arm - and using the 14.1RC CLT.

The problem is that the SDK headers are marking the sprintf function as "deprecated" which causes the GCC bootstrap to fail because -Werror.

i have checked in the places available to me (C23 draft n3047 and also in the C11 DIS) and on the posix online resources -- AFAICT there is no official deprecation of this function and so Apple are effectively rejecting standard-conforming code .. which will likely break a lot of people.

It is perfectly reasonable for the compiler to emit a warning about these functions, but not to add a deprecation unless/until WG41 and/or Posix do so.

@iains
Copy link
Owner Author

iains commented Oct 19, 2022

I see the message in the headers, though, for the macOS 13 SDK:

__swift_unavailable("Use snprintf instead.")
#if !defined(_POSIX_C_SOURCE)
__deprecated_msg("This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead.")
#endif
int      sprintf(char * __restrict, const char * __restrict, ...) __printflike(2, 3);

Apparently it was already there in prior betas: dotnet/runtime#71561

Hmm so something else has changed that presumably affects the state of _POSIX_C_SOURCE sigh not the first time this has been a hassle. I will recheck the 14.1b3 but fairly sure that was OK.

@iains
Copy link
Owner Author

iains commented Oct 19, 2022

TBH I am not sure how the claim of "compatibility only" matches up to supplying a function mandated by C and Posix stds.

@iains
Copy link
Owner Author

iains commented Oct 19, 2022

I see the message in the headers, though, for the macOS 13 SDK:

__swift_unavailable("Use snprintf instead.")
#if !defined(_POSIX_C_SOURCE)
__deprecated_msg("This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead.")
#endif
int      sprintf(char * __restrict, const char * __restrict, ...) __printflike(2, 3);

Apparently it was already there in prior betas: dotnet/runtime#71561

Hmm so something else has changed that presumably affects the state of _POSIX_C_SOURCE sigh not the first time this has been a hassle. I will recheck the 14.1b3 but fairly sure that was OK.

OK so it does also fail on Arm64 for 14.1b3. I guess we will need to figure out what has to be set to enable _POSIX_C_SOURCE ..

Unfortunately not had an arm box to test on during these last transitions .. just got to it yesterday (when reviewing open issues)

@iains
Copy link
Owner Author

iains commented Oct 19, 2022

FWIW: building with

STAGE1_CFLAGS="-Wno-deprecated-declarations" STAGE1_CXXFLAGS="-Wno-deprecated-declarations" BOOT_CFLAGS="-O2 -g -Wno-deprecated-declarations"

is a messy, but usable work-around.

@iains
Copy link
Owner Author

iains commented Oct 19, 2022

for the record 12.6 Intel also fails with 14.1RC on master .. now I need to figure out what actually worked for testing the 14.1b3 .. because we surely did check that it fixed the problem reported for GCC12.

@iains
Copy link
Owner Author

iains commented Oct 19, 2022

for the record 12.6 Intel also fails with 14.1RC on master .. now I need to figure out what actually worked for testing the 14.1b3 .. because we surely did check that it fixed the problem reported for GCC12.

12.2darwin-pre-1 does build .. so the problem could well be new to master

@iains
Copy link
Owner Author

iains commented Oct 19, 2022

for the record 12.6 Intel also fails with 14.1RC on master .. now I need to figure out what actually worked for testing the 14.1b3 .. because we surely did check that it fixed the problem reported for GCC12.

12.2darwin-pre-1 does build .. so the problem could well be new to master

  • on both Intel and Arm64 12.6 12.2-darwin-pre-r1 is OK.

  • unpatched master has the problem on x86_64 (so it's not associated with the darwin-specific additions for arm 64etc).

  • even if this is something we can fix, it seems rather bold for a vendor to make something deprecated that is mandated by upstream standards, even if partially guarded [the function is not a posix-specific thing, it is described in the C standard itself].

@iains
Copy link
Owner Author

iains commented Oct 21, 2022

(having made an initial assessment) removing all uses of sprintf () from GCC is a non-trivial project [it might well be both acceptable, and welcomed, but it will take some work, likely too much to be done in GCC-13]
So, in the short term, we need to figure out why _POSIX_C_SOURCE is not being defined for master, but apparently is for earlier branches (there can be non-obvious reasons like changes in the default checking on the branches c.f. master).

@iains
Copy link
Owner Author

iains commented Oct 22, 2022

So on Darwin21 (macOS 12) the CLT 14.1RC installs with MacOSX.sdk -> MacOSX13.0.sdk

(usually, the install on an older OS version points to the correct SDK for the version - so, in this case, one might expect MacOSX.sdk -> MacOSX12.sdk)

Sometimes there have been issues with Availability macros and building with a newer SDK .. so ...

  • I did a build current master on Darwin21, using the MacOS12.sdk it works as normal. (not surprising, since the deprecation message is not present, see below)

  • I tried building (on Darwin21) for Darwin22 using the MacOS13.sdk but that fails with the sprintf deprecations.

  • I do not have enough hardware to install Darwin22 to see if a native build there is OK (but I have now pushed the branch tested, so if some kind soul with a macOS 13 / Darwin22 installation could test, that would be great).

I thought someone reported that this function had been deprecated in earlier SDKs - but it does not seem to be in MacOSX12.sdk

@iains
Copy link
Owner Author

iains commented Oct 22, 2022

hmm this is gradually becoming more complex.

taking p.c/cc as

#include <stdio.h>
void foo()
{
   char buf[20];
  (void)sprintf (buf, "hello");
}

If we compile this with:

  • clang - no problem
  • clang++ - we get the deprecation notice (of course we build GCC with c++ so boom ...)

edit: checked the C++23 CD, there's no deprecation note associated with sprintf (the contents of <cstdio> are said to be the same as <stdio.h>).

The only difference in using .c or .cc is the warning about building C as C++.


!!!
Also it seems that latest clang from Xcode 14.1RC no longer honours --sysroot= which could cause us some issues too (it does honour -isysroot) ..

Xcode 13.4 CLT does honour --sysroot= , so that is a regression.


right now, I did not succeed yet in tracking down what is happening differently (or even where _POSIX_C_SOURCE is getting set definitively) ...
(it could be a problem with fix includes, of course -- but the fact that I can repeat this with no GCC involved makes me think we have an underlying issue)

@iains
Copy link
Owner Author

iains commented Oct 22, 2022

OK so (i think) explained the reason that "C" does not give this warning; it is because we default to fortified sources, which causes us to use sprintf_chk instead of sprintf.

clang p.c -isysroot /XC/14.1RC/CommandLineTools/SDKs/MacOSX13.sdk -S -Wdeprecated-declarations -D_FORTIFY_SOURCE=0

Fails too.

@iains
Copy link
Owner Author

iains commented Oct 22, 2022

OK so it seems that three factors and IMO a genuine bug together have led to confusion.

  1. When GCC-12 was master (and therefore built with -Werror), there was no macOS13 SDK, so these functions were not reported as deprecated.

  2. Now we build GCC-12 as a release branch using the new SDK Werror is off (we actually see the deprecations from the new SDK but they are warnings)

  3. Now we try to build GCC-13 with Werror on (that's the default for stage2 & 3) and the new SDK (with the functions deprecated so we get a fail).

for example:
GCC-12 (current darwin 12.2-pre-1):

 -g -O2 -fchecking=1 -W -Wall -Wno-narrowing -Wwrite-strings -Wmissing-format-attribute -pedantic -Wno-long-long  -fno-exceptions -fno-rtti -I/src-local/gcc-git-12/libcpp -I. -I/src-local/gcc-git-12/libcpp/../include -I./../intl -I/src-local/gcc-git-12/libcpp/include    -c -o charset.o -MT charset.o -MMD -MP -MF .deps/charset.Tpo /src-local/gcc-git-12/libcpp/charset.cc

GCC-13 (master):

-g -O2 -fno-checking -gtoggle -W -Wall -Wno-narrowing -Wwrite-strings -Wmissing-format-attribute -pedantic -Wno-long-long -Werror -fno-exceptions -fno-rtti -I/src-local/gcc-master/libcpp -I. -I/src-local/gcc-master/libcpp/../include -I./../intl -I/src-local/gcc-master/libcpp/include    -c -o charset.o -MT charset.o -MMD -MP -MF .deps/charset.Tpo /src-local/gcc-master/libcpp/charset.cc

@fxcoudert - IDK if any of your lines of contact could help get this sorted?

The bug:

The code in #98 (comment) is standard-compliant C++ and should compile without any warnings for:

clang++ -std=c++xx (for all current xx and 23 draft).

it does not.

We should try to get this fixed ASAP .. because there really is no simple way to work around it.
The code compiles without warning for clang - but IMO that's purely an accident because of the default to FORTIFY_SOURCE.

The deprecations should be "opt-in" on some "clean up my sources" option...

@fxcoudert
Copy link
Contributor

fxcoudert commented Oct 22, 2022

I think at this stage it is unlikely to be fixed. I got confirmation that the deprecation warning, even for standard code, was a deliberate decision. 😢

@iains
Copy link
Owner Author

iains commented Oct 22, 2022

I think at this stage it is unlikely to be fixed. I confirmed that the deprecation warning was a deliberate decision. 😢

Then, AFAICT, that decision makes the implementation non-conforming :(.

BOOT_CFLAGS="-O2 -g -Wno-error=deprecated-declarations"
is the least invasive change I can see .. perhaps we can fix configure to add that automatically ... but frankly it sucks.

@thesamesam
Copy link
Contributor

thesamesam commented Nov 3, 2022

@jakubjelinek
Copy link
Contributor

In gcc we can fixinclude it.

@iains
Copy link
Owner Author

iains commented Nov 9, 2022

yeah, I will look into that as soon as the Xcode 14.1 release is made (I'm away from most of my infrastructure right now) .. fixincludes is sad, but necessary sometimes.

@iains
Copy link
Owner Author

iains commented Dec 13, 2022

since this effects upstream Darwin, I am closing here and tracking should be done in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107568

@iains iains closed this as completed Dec 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants