Re: [PATCH] libstdc++: Implement C++26 features (P2546R5)
3 Jun 2024 4:14:28 pm Jonathan Wakely :
On Mon, 3 Jun 2024 at 14:36, Peter0x44 wrote:
+void
+std::breakpoint() noexcept
+{
+#if _GLIBCXX_HAVE_DEBUGAPI_H && defined(_WIN32) &&
!defined(__CYGWIN__)
+ DebugBreak();
+#elif __has_builtin(__builtin_debugtrap)
+ __builtin_debugtrap(); // Clang
+#elif defined(__i386__) || defined(__x86_64__)
+ __asm__ volatile ("int3");
Worth noting, currently gcc has some bugs around its handling of int3,
s/gcc/gdb/ ?
Yes, my bad
https://sourceware.org/bugzilla/show_bug.cgi?id=31194
int3;nop seems to work around it okay. __builtin_debugtrap() of clang
does run into the same issues.
It seemed to work OK for me, maybe because there's no code after it?
I suspect it won't matter for the tests, the assertion failure is only if
you step after hitting the int3. I just figured I'd mention it as a heads
up. It would affect users writing code.
void breakpoint() {
__asm__ volatile ("int3);
}
Re: [PATCH] libstdc++: Implement C++26 features (P2546R5)
On 2024-06-01 03:22, Jonathan Wakely wrote: Here's an implementation of the C++26 header. We should really have some tests that invoke GDB and check that the breakpoint works when a debugger is attached. That seems tricky to do via the main conformance.exp testsuite. It could be done via the prettyprinters.exp testsuite, which already uses GDB, but would require some changes to the gdb-test procedure, which assumes it needs to insert its own breakpoint at marked spots in the code. I think we could add this without those tests, and improve it later. -- >8 -- It would be good to provide a macOS definition of is_debugger_present, but that isn't included in this change. libstdc++-v3/ChangeLog: * config.h.in: Regenerate. * configure: Regenerate. * configure.ac: Check for facilities needed by . * include/Makefile.am: Add new header. * include/Makefile.in: Regenerate. * include/bits/version.def (debugging): Add. * include/bits/version.h: Regenerate. * src/c++26/Makefile.am: Add new file. * src/c++26/Makefile.in: Regenerate. * include/std/debugging: New file. * src/c++26/debugging.cc: New file. * testsuite/19_diagnostics/debugging/breakpoint.cc: New test. * testsuite/19_diagnostics/debugging/breakpoint_if_debugging.cc: New test. --- libstdc++-v3/config.h.in | 9 ++ libstdc++-v3/configure| 22 +++ libstdc++-v3/configure.ac | 9 ++ libstdc++-v3/include/Makefile.am | 1 + libstdc++-v3/include/Makefile.in | 1 + libstdc++-v3/include/bits/version.def | 9 ++ libstdc++-v3/include/bits/version.h | 10 ++ libstdc++-v3/include/std/debugging| 82 libstdc++-v3/src/c++26/Makefile.am| 4 +- libstdc++-v3/src/c++26/Makefile.in| 7 +- libstdc++-v3/src/c++26/debugging.cc | 126 ++ .../19_diagnostics/debugging/breakpoint.cc| 9 ++ .../debugging/breakpoint_if_debugging.cc | 9 ++ 13 files changed, 295 insertions(+), 3 deletions(-) create mode 100644 libstdc++-v3/include/std/debugging create mode 100644 libstdc++-v3/src/c++26/debugging.cc create mode 100644 libstdc++-v3/testsuite/19_diagnostics/debugging/breakpoint.cc create mode 100644 libstdc++-v3/testsuite/19_diagnostics/debugging/breakpoint_if_debugging.cc diff --git a/libstdc++-v3/config.h.in b/libstdc++-v3/config.h.in index 486ba450749..07203815459 100644 --- a/libstdc++-v3/config.h.in +++ b/libstdc++-v3/config.h.in @@ -70,6 +70,9 @@ /* Define to 1 if you have the `cosl' function. */ #undef HAVE_COSL +/* Define to 1 if you have the header file. */ +#undef HAVE_DEBUGAPI_H + /* Define to 1 if you have the declaration of `strnlen', and to 0 if you don't. */ #undef HAVE_DECL_STRNLEN @@ -436,6 +439,9 @@ /* Define to 1 if you have the header file. */ #undef HAVE_SYS_PARAM_H +/* Define to 1 if you have the header file. */ +#undef HAVE_SYS_PTRACE_H + /* Define to 1 if you have the header file. */ #undef HAVE_SYS_RESOURCE_H @@ -847,6 +853,9 @@ /* Define if nl_langinfo_l should be used for std::text_encoding. */ #undef _GLIBCXX_USE_NL_LANGINFO_L +/* Define if /proc/self/status should be used for . */ +#undef _GLIBCXX_USE_PROC_SELF_STATUS + /* Define if pthreads_num_processors_np is available in . */ #undef _GLIBCXX_USE_PTHREADS_NUM_PROCESSORS_NP diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure index 5645e991af7..55ddf36a7f1 100755 --- a/libstdc++-v3/configure +++ b/libstdc++-v3/configure @@ -54612,6 +54612,28 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu +# For std::is_debugger_present +case "$target_os" in + linux*) + +$as_echo "#define _GLIBCXX_USE_PROC_SELF_STATUS 1" >>confdefs.h + +;; +esac +for ac_header in sys/ptrace.h debugapi.h +do : + as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh` +ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header" "$ac_includes_default" +if eval test \"x\$"$as_ac_Header"\" = x"yes"; then : + cat >>confdefs.h <<_ACEOF +#define `$as_echo "HAVE_$ac_header" | $as_tr_cpp` 1 +_ACEOF + +fi + +done + + # Define documentation rules conditionally. # See if makeinfo has been installed and is modern enough diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac index ccb24a82be7..96b412fb7ae 100644 --- a/libstdc++-v3/configure.ac +++ b/libstdc++-v3/configure.ac @@ -573,6 +573,15 @@ GLIBCXX_CHECK_FILEBUF_NATIVE_HANDLES # For std::text_encoding GLIBCXX_CHECK_TEXT_ENCODING +# For std::is_debugger_present +case "$target_os" in + linux*) +AC_DEFINE([_GLIBCXX_USE_PROC_SELF_STATUS],1, + [Define if /proc/self/status should be used for .]) +;; +esac +AC_CHECK_HEADERS([sys/ptrace.h debugapi.h]) + # Define documentation rules conditionally. # See if makeinfo has been installed and is modern enough diff --git
Re: [PATCH] .gitattributes: disable crlf translation
On 2024-05-23 05:01, Richard Biener wrote: On Thu, May 23, 2024 at 5:50 AM Peter Damianov wrote: By default, git has the "autocrlf" """feature""" enabled. This causes the files to have CRLF line endings when checked out on windows, which in the case of configure, causes confusing errors like: ./gcc/configure: line 14: $'\r': command not found ./gcc/configure: line 29: syntax error near unexpected token `newline' '/gcc/configure: line 29: ` ;; when it is invoked. Any files damaged in this way can be fixed with: $ git config core.autocrlf false $ git reset $ git checkout . But, it's better to simply avoid this problem in the first place. This behavior is never helpful or desired for gcc. For files added/edited on Windows does this then also strip the \r (upon which action?)? Otherwise I think this looks good but I'm not a git expert. From what I can tell, the \r doesn't get stripped from the files, but the commit itself acts as if it isn't there. In the working directory, if an editor introduces a CRLF it remains, but any commits created won't include it. I am finding the git documentation a bit confusing on this point though, so I'm not certain. I'm far from a git export as well. I checked and I couldn't find any CRLFs in gcc right now. I tried the commands here: https://git-scm.com/docs/gitattributes $ git add --renormalize . $ git status# Show files that will be normalized And git status showed no changes. As far as I can tell, this change is okay. I would still feel more confident if others looked at it, though. Thanks, Peter D. Richard. Signed-off-by: Peter Damianov --- .gitattributes | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitattributes b/.gitattributes index e75bfc595bf..1e116987c98 100644 --- a/.gitattributes +++ b/.gitattributes @@ -8,3 +8,6 @@ ChangeLog whitespace=indent-with-non-tab,space-before-tab,trailing-space # Use together with git config diff.md.xfuncname '^\(define.*$' # which is run by contrib/gcc-git-customization.sh too. *.md diff=md + +# Disable lf -> crlf translation on windows. +* -crlf -- 2.39.2
Re: [PATCH v3] driver: Output to a temp file; rename upon success [PR80182]
On 2024-05-16 01:29, Richard Biener wrote:
On Sun, May 12, 2024 at 3:40 PM Peter Damianov
wrote:
Currently, commands like:
gcc -o file.c -lm
will delete the user's code.
This patch makes the linker write executables to a temp file, and then
renames
the temp file if successful. This fixes the case above, but has
limitations.
The source file will still get overwritten if the link "succeeds",
such as the
case of: gcc -o file.c -lm -r
It's not perfect, but it should hopefully stop some people from
ruining their
day.
Hmm. When suggesting this I was originally hoping for this to be
implemented
in the linker so that it delays opening (and truncating) of the output
file as much as possible.
Ah, okay, I assumed you wanted it in the driver since then it could
still fix the problem for older linker versions, but it could be a
problem to sort the linker too.
If we want to do something in the compiler driver then I like the
filename based
heuristics more. v3 seems to only address the case of -o specifying
the linker
output file but of course
The filename heuristics feel like too much hacks for my liking, but
maybe I don't have a rational reason to feel that way.
I had some trouble figuring exactly which suffixes to reject, obviously
-S should not reject .s as an output file, but I still don't think I got
it all correct. I'm also a little worried, perhaps there is some weird
makefiles or configure scripts out there that do depend on this
behavior.
gcc -c t.c -o t2.c
or
gcc -S t.c -o t2.c
happily overwrite a source file as well. For these cases
heuristically rejecting
source file patterns would be better. As we've shown the rename trick
when
the link was successful doesn't fully solve the issue. And I bet some
people
will claim it isn't an issue at all ...
I don't think there is any easy or nice way to "fully solve the issue",
especially if you want to consider -c, -E, -S, etc.
One other idea for -c could be refusing to write out the object file if
there is no elf/coff/pe/macho header, but I don't like it, sounds too
complex.
That is, I do think the linker itself, as a quality of implementation
issue,
should avoid truncating the output early. In fact the BFD linker seems
to
unlink the output very early:
Agreed. I decided to try some other linkers, lld and mold both don't
have this issue.
BFD and gold do. I suppose I should open a bug for that, or investigate
myself.
24937 stat("t.c", {st_mode=S_IFREG|0644, st_size=13, ...}) = 0
24937 lstat("t.c", {st_mode=S_IFREG|0644, st_size=13, ...}) = 0
24937 unlink("t.c") = 0
24937 openat(AT_FDCWD, "t.c", O_RDWR|O_CREAT|O_TRUNC, 0666) = 3
before even opening other inputs or the default linker script.
Richard.
gcc/ChangeLog:
PR driver/80182
* gcc.cc (output_file_temp): New global variable
(driver_handle_option): Create temp file for executable output
(driver::maybe_run_linker): Rename output_file_temp to
output_file if
the linker ran successfully
Signed-off-by: Peter Damianov
---
v3: don't attempt to create temp files -> rename for -o /dev/null
gcc/gcc.cc | 53 +
1 file changed, 37 insertions(+), 16 deletions(-)
diff --git a/gcc/gcc.cc b/gcc/gcc.cc
index 830a4700a87..5e38c6e578a 100644
--- a/gcc/gcc.cc
+++ b/gcc/gcc.cc
@@ -2138,6 +2138,11 @@ static int have_E = 0;
/* Pointer to output file name passed in with -o. */
static const char *output_file = 0;
+/* We write the output file to a temp file, and rename it if linking
+ is successful. This is to prevent mistakes like: gcc -o file.c -lm
from
+ deleting the user's code. */
+static const char *output_file_temp = 0;
+
/* Pointer to input file name passed in with -truncate.
This file should be truncated after linking. */
static const char *totruncate_file = 0;
@@ -4610,10 +4615,18 @@ driver_handle_option (struct gcc_options
*opts,
#if defined(HAVE_TARGET_EXECUTABLE_SUFFIX) ||
defined(HAVE_TARGET_OBJECT_SUFFIX)
arg = convert_filename (arg, ! have_c, 0);
#endif
- output_file = arg;
+ output_file_temp = output_file = arg;
+ /* If creating an executable, create a temp file for the
output, unless
+ -o /dev/null was requested. This will later get renamed, if
the linker
+ succeeds. */
+ if (!have_c && strcmp (output_file, HOST_BIT_BUCKET) != 0)
+{
+ output_file_temp = make_temp_file ("");
+ record_temp_file (output_file_temp, false, true);
+}
/* On some systems, ld cannot handle "-o" without a space. So
split the option from its argument. */
- save_switch ("-o", 1, , validated, true);
+ save_switch ("-o", 1, _file_temp, validated, true);
return true;
case OPT_pie:
@@ -9266,22 +9279,30 @@ driver::maybe_run_linker (const char *argv0)
const
linker_was_run = (tmp != execution_count);
}
- /* If options said don't
Re: [PATCH v2 2/3] diagnostics: Don't hardcode auto_enable_urls to false for mingw hosts
13 May 2024 1:30:28 pm NightStrike : On Thu, May 9, 2024 at 1:03 PM Peter Damianov wrote: Windows terminal and mintty both have support for link escape sequences, and so auto_enable_urls shouldn't be hardcoded to false. For older versions of the windows console, mingw_ansi_fputs's console API translation logic does mangle these sequences, but there's nothing useful it could do even if this weren't the case, so check if the ansi escape sequences are supported at all. conhost.exe doesn't support link escape sequences, but printing them does not cause any problems. Are there any issues when running under the Wine console, such as when running the testsuite? I installed wine and gave compiling a file emitting a warning a try. Unfortunately, yes, gcc emits mangled warnings here. Even simply running this patch under wine causes problems, it's not just wine's conhost.exe. I'm not sure whether it's my fault or wine's. I've attached two screenshots demonstrating exactly what happens. (I think???) wine should only be advertising that it supports those settings regarding escape sequences if it actually does. Also, on this machine, wine is near unusably slow, I'm talking multiple seconds to react to a keypress through the wine conhost. I will not be attempting to run the testsuite, I severely doubt it will work.
Re: [PATCH v2 2/3] diagnostics: Don't hardcode auto_enable_urls to false for mingw hosts
13 May 2024 1:30:28 pm NightStrike : On Thu, May 9, 2024 at 1:03 PM Peter Damianov wrote: Windows terminal and mintty both have support for link escape sequences, and so auto_enable_urls shouldn't be hardcoded to false. For older versions of the windows console, mingw_ansi_fputs's console API translation logic does mangle these sequences, but there's nothing useful it could do even if this weren't the case, so check if the ansi escape sequences are supported at all. conhost.exe doesn't support link escape sequences, but printing them does not cause any problems. Are there any issues when running under the Wine console, such as when running the testsuite? I did not try this. There shouldn't be problems if wine implements ENABLE_VIRTUAL_TERMINAL_PROCESSING correctly, but I agree it would be good to check. Are there instructions anywhere for running the testsuite with wine? Anything specific I need to do?
Re: [PATCH v2 1/3] diagnostics: Enable escape sequence processing on windows consoles
9 May 2024 6:02:34 pm Peter Damianov :
Since windows 10 release v1511, the windows console has had support for
VT100
escape sequences. We should try to enable this, and utilize it where
possible.
gcc/ChangeLog:
* diagnostic-color.cc (should_colorize): Enable processing of VT100
escape sequences on windows consoles
Signed-off-by: Peter Damianov
---
Forgot to add -v2 to git send-email the first time I sent. Sorry for
the spam.
gcc/diagnostic-color.cc | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/gcc/diagnostic-color.cc b/gcc/diagnostic-color.cc
index f01a0fc2e37..3af198654af 100644
--- a/gcc/diagnostic-color.cc
+++ b/gcc/diagnostic-color.cc
@@ -213,12 +213,23 @@ should_colorize (void)
pp_write_text_to_stream() in pretty-print.cc calls fputs() on
that stream. However, the code below for non-Windows doesn't
seem
to care about it either... */
- HANDLE h;
- DWORD m;
+ HANDLE handle;
+ DWORD mode;
+ BOOL isconsole = false;
- h = GetStdHandle (STD_ERROR_HANDLE);
- return (h != INVALID_HANDLE_VALUE) && (h != NULL)
- && GetConsoleMode (h, );
+ handle = GetStdHandle (STD_ERROR_HANDLE);
+
+ if ((handle != INVALID_HANDLE_VALUE) && (handle != NULL))
+ isconsole = GetConsoleMode (handle, );
+
+ if (isconsole)
+ {
+ /* Try to enable processing of VT100 escape sequences */
+ mode |= ENABLE_PROCESSED_OUTPUT |
ENABLE_VIRTUAL_TERMINAL_PROCESSING;
+ SetConsoleMode (handle, mode);
+ }
+
+ return isconsole;
#else
char const *t = getenv ("TERM");
/* emacs M-x shell sets TERM="dumb". */
--
2.39.2
I asked a windows terminal maintainer to review the patches here:
https://github.com/microsoft/terminal/discussions/17219#discussioncomment-9375044
And got an "LGTM".
I tested the patches with windows terminal, conhost.exe, and conhost.exe
with the "use legacy console" box checked, and they all worked correctly.
I think this is okay for trunk.
Re: [PATCH] Driver: Reject output filenames with the same suffixes as source files [PR80182]
On Mon May 6, 2024 at 8:14 AM BST, Richard Biener wrote:
> On Sat, May 4, 2024 at 9:36 PM Peter Damianov wrote:
> >
> > Currently, commands like:
> > gcc -o file.c -lm
> > will delete the user's code.
>
> Since there's an error from the linker in the end (missing 'main'), I wonder
> if
> the linker can avoid truncating/opening the output file instead? A trivial
> solution might be to open a temporary file first and only atomically replace
> the output file with the temporary file when there were no errors?
I think this is a great idea! The only concern I have is that I think
for mingw targets it would be necessary to be careful to append .exe if
the file has no suffix when moving the temporary file to the output
file. Maybe some other targets have similar concerns.
>
> > This patch checks the suffix of the output, and errors if the output ends in
> > any of the suffixes listed in default_compilers.
> >
> > Unfortunately, I couldn't come up with a better heuristic to diagnose this
> > case
> > more specifically, so it is now not possible to directly make executables
> > with
> > said suffixes. I am unsure if any users are depending on this.
>
> A way to provide a workaround would be to require the file not existing. So
> change the heuristic to only trigger if the output file exists (and is
> non-empty?).
I guess this could work, and has a lower chance of breaking anyone
depending on this behavior, but I think it would still be confusing to
anyone who did rely on this behavior, since then it wouldn't be allowed
to overwrite an executable with the ".c" name. If anyone did rely on
this behavior, their build would succeed once, and then error for every
subsequent invokation, which would be confusing. It seems to me it is
not a meaningful improvement.
With your previous suggestion, this whole heuristic becomes unnecessary
anyway, so I think I will just forego it.
>
> Richard.
>
> > PR driver/80182
> > * gcc.cc (process_command): fatal_error if the output has the
> > suffix of
> > a source file.
> > (have_c): Change type to bool.
> > (have_O): Change type to bool.
> > (have_E): Change type to bool.
> > (have_S): New global variable.
> > (driver_handle_option): Assign have_S
> >
> > Signed-off-by: Peter Damianov
> > ---
> > gcc/gcc.cc | 29 ++---
> > 1 file changed, 26 insertions(+), 3 deletions(-)
> >
> > diff --git a/gcc/gcc.cc b/gcc/gcc.cc
> > index 830a4700a87..53169c16460 100644
> > --- a/gcc/gcc.cc
> > +++ b/gcc/gcc.cc
> > @@ -2127,13 +2127,16 @@ static vec at_file_argbuf;
> > static bool in_at_file = false;
> >
> > /* Were the options -c, -S or -E passed. */
> > -static int have_c = 0;
> > +static bool have_c = false;
> >
> > /* Was the option -o passed. */
> > -static int have_o = 0;
> > +static bool have_o = false;
> >
> > /* Was the option -E passed. */
> > -static int have_E = 0;
> > +static bool have_E = false;
> > +
> > +/* Was the option -S passed. */
> > +static bool have_S = false;
> >
> > /* Pointer to output file name passed in with -o. */
> > static const char *output_file = 0;
> > @@ -4593,6 +4596,10 @@ driver_handle_option (struct gcc_options *opts,
> >have_E = true;
> >break;
> >
> > +case OPT_S:
> > + have_S = true;
> > + break;
> > +
> > case OPT_x:
> >spec_lang = arg;
> >if (!strcmp (spec_lang, "none"))
> > @@ -5058,6 +5065,22 @@ process_command (unsigned int decoded_options_count,
> >output_file);
> > }
> >
> > + /* Reject output file names that have the same suffix as a source
> > + file. This is to catch mistakes like: gcc -o file.c -lm
> > + that could delete the user's code. */
> > + if (have_o && output_file != NULL && !have_E && !have_S)
> > +{
> > + const char* filename = lbasename(output_file);
> > + const char* suffix = strchr(filename, '.');
> > + if (suffix != NULL)
> > + for (int i = 0; i < n_default_compilers; ++i)
> > + if (!strcmp(suffix, default_compilers[i].suffix))
> > + fatal_error (input_location,
> > +"output file suffix %qs could be a source file",
> > +suffix);
> > +}
> > +
> > +
> >if (output_file != NULL && output_file[0] == '\0')
> > fatal_error (input_location, "output filename may not be empty");
> >
> > --
> > 2.39.2
> >
Thanks for the feedback,
Peter D.
Re: [PATCH v3 1/2] Driver: Add new -truncate option
29 Apr 2024 12:16:26 am Peter Damianov :
This commit adds a new option to the driver that truncates one file
after
linking.
Tested likeso:
$ gcc hello.c -c
$ du -h hello.o
4.0K hello.o
$ gcc hello.o -truncate hello.o
$ ./a.out
Hello world
$ du -h hello.o
$ 0 hello.o
$ gcc hello.o -truncate
gcc: error: missing filename after '-truncate'
The motivation for adding this is PR110710. It is used by lto-wrapper
to
truncate files in a shell-independent manner.
Signed-off-by: Peter Damianov
---
gcc/common.opt | 6 ++
gcc/gcc.cc | 14 ++
2 files changed, 20 insertions(+)
diff --git a/gcc/common.opt b/gcc/common.opt
index ad348844775..40cab3cb36a 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -422,6 +422,12 @@ Display target specific command line options
(including assembler and linker opt
-time
Driver Alias(time)
+;; Truncate the file specified after linking.
+;; This option is used by lto-wrapper to reduce the peak disk-usage
when
+;; linking with many .LTRANS units.
+truncate
+Driver Separate Undocumented MissingArgError(missing filename after
%qs)
+
-verbose
Driver Alias(v)
diff --git a/gcc/gcc.cc b/gcc/gcc.cc
index 728332b8153..830a4700a87 100644
--- a/gcc/gcc.cc
+++ b/gcc/gcc.cc
@@ -2138,6 +2138,10 @@ static int have_E = 0;
/* Pointer to output file name passed in with -o. */
static const char *output_file = 0;
+/* Pointer to input file name passed in with -truncate.
+ This file should be truncated after linking. */
+static const char *totruncate_file = 0;
+
/* This is the list of suffixes and codes (%g/%u/%U/%j) and the
associated
temp file. If the HOST_BIT_BUCKET is used for %j, no entry is made
for
it here. */
@@ -4538,6 +4542,11 @@ driver_handle_option (struct gcc_options *opts,
do_save = false;
break;
+ case OPT_truncate:
+ totruncate_file = arg;
+ do_save = false;
+ break;
+
case OPT:
/* "-###"
This is similar to -v except that there is no execution
@@ -9286,6 +9295,11 @@ driver::final_actions () const
delete_failure_queue ();
delete_temp_files ();
+ if (totruncate_file != NULL && !seen_error ())
+ /* Truncate file specified by -truncate.
+ Used by lto-wrapper to reduce temporary disk-space usage. */
+ truncate(totruncate_file, 0);
+
if (print_help_list)
{
printf (("\nFor bug reporting instructions, please see:\n"));
--
2.39.2
I resubmitted the patch because the previous one had a mistake.
It didn't set "do_save" to false, so it resulted in problems like this:
./gcc/xgcc -truncate
xgcc: error: missing filename after ‘-truncate’
xgcc: fatal error: no input files
./gcc/xgcc -truncate ??
xgcc: error: unrecognized command-line option ‘-truncate’
xgcc: fatal error: no input files
Therefore regressing some tests, and not working properly.
After fixing this, I ran all of the LTO tests again and observed no
failures.
I'm not sure how I ever observed it working before, but I'm reasonably
confident this is correct now.
Re: [PATCH v2 1/2] Driver: Add new -truncate option
18 Apr 2024 7:26:27 am Richard Biener :
On Thu, Apr 18, 2024 at 6:12 AM Peter Damianov
wrote:
This commit adds a new option to the driver that truncates one file
after
linking.
Tested likeso:
$ gcc hello.c -c
$ du -h hello.o
4.0K hello.o
$ gcc hello.o -truncate hello
$ ./a.out
Hello world
$ du -h hello.o
$ 0 hello.o
I suppose it should have been
$ gcc hello.o -truncate hello.o
in the example.
Correct.
Sorry about that.
$ gcc hello.o -truncate
gcc: error: missing filename after '-truncate'
The motivation for adding this is PR110710. It is used by lto-wrapper
to
truncate files in a shell-independent manner.
This looks good to me.
Thanks,
Richard.
Signed-off-by: Peter Damianov
---
v2: moved truncation to driver::final_actions
v2: moved handling of OPT_truncate to be in alphabetic order
gcc/common.opt | 6 ++
gcc/gcc.cc | 13 +
2 files changed, 19 insertions(+)
diff --git a/gcc/common.opt b/gcc/common.opt
index ad348844775..40cab3cb36a 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -422,6 +422,12 @@ Display target specific command line options
(including assembler and linker opt
-time
Driver Alias(time)
+;; Truncate the file specified after linking.
+;; This option is used by lto-wrapper to reduce the peak disk-usage
when
+;; linking with many .LTRANS units.
+truncate
+Driver Separate Undocumented MissingArgError(missing filename after
%qs)
+
-verbose
Driver Alias(v)
diff --git a/gcc/gcc.cc b/gcc/gcc.cc
index 728332b8153..b4169bbd3be 100644
--- a/gcc/gcc.cc
+++ b/gcc/gcc.cc
@@ -2138,6 +2138,10 @@ static int have_E = 0;
/* Pointer to output file name passed in with -o. */
static const char *output_file = 0;
+/* Pointer to input file name passed in with -truncate.
+ This file should be truncated after linking. */
+static const char *totruncate_file = 0;
+
/* This is the list of suffixes and codes (%g/%u/%U/%j) and the
associated
temp file. If the HOST_BIT_BUCKET is used for %j, no entry is
made for
it here. */
@@ -4538,6 +4542,10 @@ driver_handle_option (struct gcc_options *opts,
do_save = false;
break;
+ case OPT_truncate:
+ totruncate_file = arg;
+ break;
+
case OPT:
/* "-###"
This is similar to -v except that there is no execution
@@ -9286,6 +9294,11 @@ driver::final_actions () const
delete_failure_queue ();
delete_temp_files ();
+ if (totruncate_file != NULL && !seen_error ())
+ /* Truncate file specified by -truncate.
+ Used by lto-wrapper to reduce temporary disk-space usage. */
+ truncate(totruncate_file, 0);
+
if (print_help_list)
{
printf (("\nFor bug reporting instructions, please see:\n"));
--
2.39.2
Re: [PATCH 1/2] Driver: Add new -truncate option
On 2024-04-17 17:56, Peter Damianov wrote:
This commit adds a new option to the driver that truncates one file
after
linking.
Tested likeso:
$ gcc hello.c -c
$ du -h hello.o
4.0K hello.o
$ gcc hello.o -truncate hello
$ ./a.out
Hello world
$ du -h hello.o
$ 0 hello.o
$ gcc hello.o -truncate
gcc: error: missing filename after '-truncate'
The motivation for adding this is PR110710. It is used by lto-wrapper
to
truncate files in a shell-independent manner.
Signed-off-by: Peter Damianov
---
gcc/common.opt | 5 +
gcc/gcc.cc | 13 +
2 files changed, 18 insertions(+)
diff --git a/gcc/common.opt b/gcc/common.opt
index ad348844775..3ede2fa8552 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -422,6 +422,11 @@ Display target specific command line options
(including assembler and linker opt
-time
Driver Alias(time)
+;; Truncate the file specified after linking.
+;; This option is used by lto-wrapper to reduce the peak disk when
linking with
+;; many .LTRANS units.
+Driver Separate Undocumented MissingArgError(missing filename after
%qs)
+
-verbose
Driver Alias(v)
diff --git a/gcc/gcc.cc b/gcc/gcc.cc
index 728332b8153..00017964295 100644
--- a/gcc/gcc.cc
+++ b/gcc/gcc.cc
@@ -2138,6 +2138,10 @@ static int have_E = 0;
/* Pointer to output file name passed in with -o. */
static const char *output_file = 0;
+/* Pointer to input file name passed in with -truncate.
+ This file should be truncated after linking. */
+static const char *totruncate_file = 0;
+
/* This is the list of suffixes and codes (%g/%u/%U/%j) and the
associated
temp file. If the HOST_BIT_BUCKET is used for %j, no entry is made
for
it here. */
@@ -4607,6 +4611,10 @@ driver_handle_option (struct gcc_options *opts,
save_switch ("-o", 1, , validated, true);
return true;
+case OPT_truncate:
+ totruncate_file = arg;
+ break;
+
case OPT_pie:
#ifdef ENABLE_DEFAULT_PIE
/* -pie is turned on by default. */
@@ -9273,6 +9281,11 @@ driver::maybe_run_linker (const char *argv0)
const
option). */
error ("%s: linker input file not found: %m", outfiles[i]);
}
+
+ if (totruncate_file != NULL && linker_was_run && !seen_error ())
+/* Truncate file specified by -truncate.
+ Used by lto-wrapper to reduce temporary disk-space usage. */
+truncate(totruncate_file, 0);
On second thought, doing the truncation in driver::maybe_run_linker()
seems wrong.
driver::final_actions seems like the better place to put this code.
Will resubmit.
}
/* The end of "main". */
Re: [PATCH] lto: Don't assume a posix shell is usable on windows [PR110710]
>> > Another way would be to have a portable solution to truncate a file >> > (maybe even removing it would work). I don't think we should override >> > SHELL. I've been thinking harder about this, these files get unlinked at the end if they are temporary. Is there no way to get make to communicate back this info so lto-wrapper can just unlink the file on its own? It feels easier and less invasive than introducing a whole new option to the driver for only that purpose. If there is some way, I'm not aware of it.
Re: [PATCH] lto: Don't assume a posix shell is usable on windows [PR110710]
On 2024-03-27 01:58, Richard Biener wrote: On Wed, Mar 27, 2024 at 9:13 AM Peter0x44 wrote: I accidentally replied off-list. Sorry. 27 Mar 2024 8:09:30 am Peter0x44 : 27 Mar 2024 7:51:26 am Richard Biener : > On Tue, Mar 26, 2024 at 11:37 PM Peter Damianov > wrote: >> >> lto-wrapper generates Makefiles that use the following: >> touch -r file file.tmp && mv file.tmp file >> to truncate files. >> If there is no suitable "touch" or "mv" available, then this errors >> with >> "The system cannot find the file specified". >> >> The solution here is to check if sh -c true works, before trying to >> use it in >> the Makefile. If it doesn't, then fall back to "copy /y nul file" >> instead. >> The fallback doesn't work exactly the same (the modified time of the >> file gets >> updated), but this doesn't seem to matter in practice. > > I suppose it doesn't matter as we (no longer?) have the input as > dependency > on the rule so make doesn't get confused to re-build it. I guess we > only truncate > the file because it's going to be deleted by another process. > > Instead of doing sth like sh_exists I would suggest to simply use > #ifdef __WIN > or something like that? Not sure if we have suitable macros to > identify the > host operating system though and whether mingw, cygwin, etc. behave the > same > here. They do, I tested. Using sh_exists is deliberate, I've had to program on school computers that had cmd.exe disabled, but had busybox-w32 working, so it might be more flexible in that way. I would prefer a solution which didn't require invoking cmd.exe if there is a working shell present. Hmm, but then I'd expect SHELL to be appropriately set in such situation? So shouldn't sh_exists at least try to look at $SHELL? I'm not sure it would . On windows, make searches the PATH for an sh.exe if present, and then uses it by default. The relevant code is here: https://git.savannah.gnu.org/cgit/make.git/tree/src/variable.c#n1628 I figured doing the "sh_exists" is okay because there is a basically identical check for make. > > As a stop-gap solution doing > > ( @-touch -r ... ) || true > > might also work? Or another way to note to make the command can fail > without causing a problem. I don't think it would work. cmd.exe can't run subshells like this. Hmm, OK. So this is all for the case where 'make' is available (as you say we check for that) but no POSIX command environment is (IIRC both touch and mv are part of POSIX). Testing for 'touch' and 'mv' would then be another option? I think it would work, but keep in mind they could be placed on the PATH but still invoked from cmd.exe, so you might have to be careful of the redirection syntax and no /dev/null. It's a bit more complexity that doesn't seem necessary, to me. > > Another way would be to have a portable solution to truncate a file > (maybe even removing it would work). I don't think we should override > SHELL. Do you mean that perhaps an special command line argument could be added to lto-wrapper to do it, and then the makefile could invoke lto-wrapper to remove or truncate files instead of a shell? I'm not sure I get the proposed suggestion. The point is to truncate the file at the earliest point to reduce the peak disk space required for a LTO link with many LTRANS units. But yes, I guess it would be possible to add a flag to gcc itself so the LTRANS compile would truncate the file. Currently we emit sth like ./libquantum.ltrans0.ltrans.o: @/space/rguenther/install/trunk-r14-8925/usr/local/bin/gcc '-xlto' '-c' '-fno-openmp' '-fno-openacc' '-fno-pie' '-fcf-protection=none' '-g' '-mtune=generic' '-march=x86-64' '-O2' '-O2' '-g' '-v' '-save-temps' '-mtune=generic' '-march=x86-64' '-dumpdir' 'libquantum.' '-dumpbase' './libquantum.ltrans0.ltrans' '-fltrans' '-o' './libquantum.ltrans0.ltrans.o' './libquantum.ltrans0.o' so adding a '-truncate-input' flag for the driver or even more explicit '-truncate ./libquantum.ltrans0.o' might be a more elegant solution then? This is much more elegant. I like it, I can take a look at implementing it. Would I be looking at gcc.cc for this? Best wishes, Peter. Richard. > > Richard. > >> I tested this both in environments both with and without sh present, >> and >> observed no issues. >> >> Signed-off-by: Peter Damianov >> --- >> gcc/lto-wrapper.cc | 35 --- >> 1 file changed, 32 insertions(+), 3 deletions(-) >> >> diff --git a/gcc/lto-wrapper.cc b/gcc/lto-wrapper.cc >> index 5186d040ce0..8dee0aaa2d8 100644 >> --- a/gcc/lto-wrapper.cc >> +++ b/gcc/lto-wrapper.cc >> @@ -1389,6 +1389,27 @@ make_exists (void) >&g
Re: [PATCH] lto: Don't assume a posix shell is usable on windows [PR110710]
I accidentally replied off-list. Sorry.
27 Mar 2024 8:09:30 am Peter0x44 :
27 Mar 2024 7:51:26 am Richard Biener :
On Tue, Mar 26, 2024 at 11:37 PM Peter Damianov
wrote:
lto-wrapper generates Makefiles that use the following:
touch -r file file.tmp && mv file.tmp file
to truncate files.
If there is no suitable "touch" or "mv" available, then this errors
with
"The system cannot find the file specified".
The solution here is to check if sh -c true works, before trying to
use it in
the Makefile. If it doesn't, then fall back to "copy /y nul file"
instead.
The fallback doesn't work exactly the same (the modified time of the
file gets
updated), but this doesn't seem to matter in practice.
I suppose it doesn't matter as we (no longer?) have the input as
dependency
on the rule so make doesn't get confused to re-build it. I guess we
only truncate
the file because it's going to be deleted by another process.
Instead of doing sth like sh_exists I would suggest to simply use
#ifdef __WIN
or something like that? Not sure if we have suitable macros to
identify the
host operating system though and whether mingw, cygwin, etc. behave the
same
here.
They do, I tested. Using sh_exists is deliberate, I've had to program on
school computers that had cmd.exe disabled, but had busybox-w32 working,
so it might be more flexible in that way. I would prefer a solution which
didn't require invoking cmd.exe if there is a working shell present.
I figured doing the "sh_exists" is okay because there is a basically
identical check for make.
As a stop-gap solution doing
( @-touch -r ... ) || true
might also work? Or another way to note to make the command can fail
without causing a problem.
I don't think it would work. cmd.exe can't run subshells like this.
Another way would be to have a portable solution to truncate a file
(maybe even removing it would work). I don't think we should override
SHELL.
Do you mean that perhaps an special command line argument could be added
to lto-wrapper to do it, and then the makefile could invoke lto-wrapper
to remove or truncate files instead of a shell? I'm not sure I get the
proposed suggestion.
Richard.
I tested this both in environments both with and without sh present,
and
observed no issues.
Signed-off-by: Peter Damianov
---
gcc/lto-wrapper.cc | 35 ---
1 file changed, 32 insertions(+), 3 deletions(-)
diff --git a/gcc/lto-wrapper.cc b/gcc/lto-wrapper.cc
index 5186d040ce0..8dee0aaa2d8 100644
--- a/gcc/lto-wrapper.cc
+++ b/gcc/lto-wrapper.cc
@@ -1389,6 +1389,27 @@ make_exists (void)
return errmsg == NULL && exit_status == 0 && err == 0;
}
+/* Test that an sh command is present and working, return true if so.
+ This is only relevant for windows hosts, where a /bin/sh shell
cannot
+ be assumed to exist. */
+
+static bool
+sh_exists (void)
+{
+ const char *sh = "sh";
+ const char *sh_args[] = {sh, "-c", "true", NULL};
+#ifdef _WIN32
+ int exit_status = 0;
+ int err = 0;
+ const char *errmsg
+ = pex_one (PEX_SEARCH, sh_args[0], CONST_CAST (char **, sh_args),
+ "sh", NULL, NULL, _status, );
+ return errmsg == NULL && exit_status == 0 && err == 0;
+#else
+ return true;
+#endif
+}
+
/* Execute gcc. ARGC is the number of arguments. ARGV contains the
arguments. */
static void
@@ -1402,6 +1423,7 @@ run_gcc (unsigned argc, char *argv[])
const char *collect_gcc;
char *collect_gcc_options;
int parallel = 0;
+ bool have_sh = sh_exists ();
int jobserver = 0;
bool jobserver_requested = false;
int auto_parallel = 0;
@@ -2016,6 +2038,7 @@ cont:
argv_ptr[5] = NULL;
if (parallel)
{
+ fprintf (mstream, "SHELL=%s\n", have_sh ? "sh" : "cmd");
fprintf (mstream, "%s:\n\t@%s ", output_name,
new_argv[0]);
for (j = 1; new_argv[j] != NULL; ++j)
fprintf (mstream, " '%s'", new_argv[j]);
@@ -2024,9 +2047,15 @@ cont:
truncate them as soon as we have processed it. This
reduces temporary disk-space usage. */
if (! save_temps)
- fprintf (mstream, "\t@-touch -r \"%s\" \"%s.tem\" >
/dev/null "
- "2>&1 && mv \"%s.tem\" \"%s\"\n",
- input_name, input_name, input_name,
input_name);
+ {
+ fprintf (mstream,
+ have_sh
+ ? "\t@-touch -r \"%s\" \"%s.tem\" >
/dev/null "
+ "2>&1 && mv \"%s.tem\" \"%s\"\n"
+ : "\t@-copy /y nul \"%s\" > NUL "
+ "2>&1\n",
+ input_name, input_name, input_name,
input_name);
+ }
}
else
{
--
2.39.2
Re: [PATCH] lto: Don't assume a posix shell is usable on windows [PR110710]
26 Mar 2024 10:36:45 pm Peter Damianov :
lto-wrapper generates Makefiles that use the following:
touch -r file file.tmp && mv file.tmp file
to truncate files.
If there is no suitable "touch" or "mv" available, then this errors
with
"The system cannot find the file specified".
The solution here is to check if sh -c true works, before trying to use
it in
the Makefile. If it doesn't, then fall back to "copy /y nul file"
instead.
The fallback doesn't work exactly the same (the modified time of the
file gets
updated), but this doesn't seem to matter in practice.
I tested this both in environments both with and without sh present,
and
observed no issues.
Signed-off-by: Peter Damianov
---
gcc/lto-wrapper.cc | 35 ---
1 file changed, 32 insertions(+), 3 deletions(-)
diff --git a/gcc/lto-wrapper.cc b/gcc/lto-wrapper.cc
index 5186d040ce0..8dee0aaa2d8 100644
--- a/gcc/lto-wrapper.cc
+++ b/gcc/lto-wrapper.cc
@@ -1389,6 +1389,27 @@ make_exists (void)
return errmsg == NULL && exit_status == 0 && err == 0;
}
+/* Test that an sh command is present and working, return true if so.
+ This is only relevant for windows hosts, where a /bin/sh shell
cannot
+ be assumed to exist. */
+
+static bool
+sh_exists (void)
+{
+ const char *sh = "sh";
+ const char *sh_args[] = {sh, "-c", "true", NULL};
+#ifdef _WIN32
+ int exit_status = 0;
+ int err = 0;
+ const char *errmsg
+ = pex_one (PEX_SEARCH, sh_args[0], CONST_CAST (char **, sh_args),
+ "sh", NULL, NULL, _status, );
+ return errmsg == NULL && exit_status == 0 && err == 0;
+#else
+ return true;
+#endif
+}
+
/* Execute gcc. ARGC is the number of arguments. ARGV contains the
arguments. */
static void
@@ -1402,6 +1423,7 @@ run_gcc (unsigned argc, char *argv[])
const char *collect_gcc;
char *collect_gcc_options;
int parallel = 0;
+ bool have_sh = sh_exists ();
int jobserver = 0;
bool jobserver_requested = false;
int auto_parallel = 0;
@@ -2016,6 +2038,7 @@ cont:
argv_ptr[5] = NULL;
if (parallel)
{
+ fprintf (mstream, "SHELL=%s\n", have_sh ? "sh" : "cmd");
fprintf (mstream, "%s:\n\t@%s ", output_name, new_argv[0]);
for (j = 1; new_argv[j] != NULL; ++j)
fprintf (mstream, " '%s'", new_argv[j]);
@@ -2024,9 +2047,15 @@ cont:
truncate them as soon as we have processed it. This
reduces temporary disk-space usage. */
if (! save_temps)
- fprintf (mstream, "\t@-touch -r \"%s\" \"%s.tem\" > /dev/null "
- "2>&1 && mv \"%s.tem\" \"%s\"\n",
- input_name, input_name, input_name, input_name);
+ {
+ fprintf (mstream,
+ have_sh
+ ? "\t@-touch -r \"%s\" \"%s.tem\" > /dev/null "
+ "2>&1 && mv \"%s.tem\" \"%s\"\n"
+ : "\t@-copy /y nul \"%s\" > NUL "
+ "2>&1\n",
+ input_name, input_name, input_name, input_name);
+ }
}
else
{
--
2.39.2
I made this patch against gcc 13.2, because I couldn't get gcc 14 to
build.
I got the following errors:
In file included from ../../../../gcc/libgcc/config/i386/cpuinfo.c:30:
../../../../gcc/libgcc/../gcc/common/config/i386/cpuinfo.h: In function
'get_available_features':
../../../../gcc/libgcc/../gcc/common/config/i386/cpuinfo.h:938:21: error:
'bit_USER_MSR' undeclared (first use in this function)
938 | if (edx & bit_USER_MSR)
| ^~~~
../../../../gcc/libgcc/../gcc/common/config/i386/cpuinfo.h:938:21: note:
each undeclared identifier is reported only once for each function it
appears in
../../../../gcc/libgcc/../gcc/common/config/i386/cpuinfo.h:950:25: error:
'bit_AVXVNNIINT16' undeclared (first use in this function); did you mean
'bit_AVXVNNIINT8'?
950 | if (edx & bit_AVXVNNIINT16)
| ^~~~
| bit_AVXVNNIINT8
Hopefully it's still okay.
