Re: [PATCH] Allow overriding -split-dwarf-file
On Friday 11 of April 2014, Eric Christopher wrote: On Fri, Apr 4, 2014 at 9:07 AM, Lubos Lunak l.lu...@centrum.cz wrote: On Friday 04 of April 2014, David Blaikie wrote: Oops, re-add cfe-commits. On Fri, Apr 4, 2014 at 8:34 AM, David Blaikie dblai...@gmail.com wrote: On Fri, Apr 4, 2014 at 8:24 AM, Lubos Lunak l.lu...@centrum.cz wrote: changing DW_AT_GNU_dwo_name explicitly after the build, but that seems needlessly complex given that this seems to be exactly what the option does. I don't see why I would be allowed to override any option except for this one. -Xclang and the underlying driver arguments aren't really a stable/guaranteed interface. I'd be more inclined to accept this change if it were just for some debugging, but since it sounds like you want to rely on it, it's good for us to understand the goal and perhaps suggest or provide the best way of achieving it long-term. It's stable/guaranteed enough for me, and I'd rather have a clean solution that maybe breaks one day than something hackish the whole time. I'm guessing you mean that the actual output file name differs? I.e. some sort of temporary cached file ala ccache that doesn't have the same name as the file that you've just compiled? Yes. Something like clang -c -g -gsplit-dwarf a.cpp may end up actually becoming something along the lines of clang -c -x c - -g -gsplit-dwarf -o /tmp/fhqtewdsg.o The directory ends up being the same here (or can be set with, as you surmised -fdebug-compilation-dir). If so, I don't mind an option to set here necessarily, but would like to see it plumbed through and not rely on Xclang options. Like this? -- Lubos Lunak From 5f6da37c00e66931dc3867d9b5c8af30c803e12a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubo=C5=A1=20Lu=C5=88=C3=A1k?= l.lu...@centrum.cz Date: Fri, 4 Apr 2014 09:12:59 +0200 Subject: [PATCH] -fdebug-compilation-dir and -fsplit-dwarf-file also in the driver This allows overriding the normal values in special cases as such distributed build systems performing the compilation in a chroot where the expected locations do not exist. --- include/clang/Driver/CC1Options.td | 4 include/clang/Driver/Options.td | 6 ++ lib/Driver/Tools.cpp| 32 lib/Frontend/CompilerInvocation.cpp | 2 +- test/CodeGen/split-debug-filename.c | 7 +-- test/Driver/debug.c | 4 test/Driver/split-debug.c | 8 +++- 7 files changed, 43 insertions(+), 20 deletions(-) diff --git a/include/clang/Driver/CC1Options.td b/include/clang/Driver/CC1Options.td index e84a20b..3f85c00 100644 --- a/include/clang/Driver/CC1Options.td +++ b/include/clang/Driver/CC1Options.td @@ -128,8 +128,6 @@ def disable_llvm_verifier : Flag[-], disable-llvm-verifier, HelpTextDon't run the LLVM IR verifier pass; def disable_red_zone : Flag[-], disable-red-zone, HelpTextDo not emit code that uses the red zone.; -def fdebug_compilation_dir : Separate[-], fdebug-compilation-dir, - HelpTextThe compilation directory to embed in the debug info.; def dwarf_debug_flags : Separate[-], dwarf-debug-flags, HelpTextThe string to embed in the Dwarf debug flags record.; def dwarf_column_info : Flag[-], dwarf-column-info, @@ -405,8 +403,6 @@ def fsjlj_exceptions : Flag[-], fsjlj-exceptions, HelpTextUse SjLj style exceptions; def main_file_name : Separate[-], main-file-name, HelpTextMain file name to use for debug info; -def split_dwarf_file : Separate[-], split-dwarf-file, - HelpTextFile name to use for split dwarf debug info output; def fno_wchar : Flag[-], fno-wchar, HelpTextDisable C++ builtin type wchar_t; def fconstant_string_class : Separate[-], fconstant-string-class, diff --git a/include/clang/Driver/Options.td b/include/clang/Driver/Options.td index 8d2e368..1477f3d 100644 --- a/include/clang/Driver/Options.td +++ b/include/clang/Driver/Options.td @@ -910,6 +910,12 @@ def fdebug_types_section: Flag [-], fdebug-types-section, Groupf_Group, Flags[CC1Option], HelpTextPlace debug types in their own section (ELF Only); def fno_debug_types_section: Flag[-], fno-debug-types-section, Groupf_Group, Flags[CC1Option]; +def fsplit_dwarf_file : Separate[-], fsplit-dwarf-file, Groupf_Group, + Flags[CC1Option], + HelpTextFile name to use for split dwarf debug info; +def fdebug_compilation_dir : Separate[-], fdebug-compilation-dir, Groupf_Group, + Flags[CC1Option], + HelpTextThe compilation directory to embed in the debug info.; def g_Flag : Flag[-], g, Groupg_Group, HelpTextGenerate source level debug information, Flags[CC1Option]; def gline_tables_only : Flag[-], gline-tables-only, Groupg_Group, diff --git a/lib/Driver/Tools.cpp b/lib/Driver/Tools.cpp index 94d13b2..2267d15 100644 --- a/lib/Driver/Tools.cpp +++ b/lib/Driver/Tools.cpp @@ -2087,6 +2087,11 @@ static bool shouldUseLeafFramePointer(const ArgList Args, /// Add a CC1 option to specify the debug
Re: [PATCH] Allow overriding -split-dwarf-file
On Fri, Apr 4, 2014 at 9:07 AM, Lubos Lunak l.lu...@centrum.cz wrote: On Friday 04 of April 2014, David Blaikie wrote: Oops, re-add cfe-commits. On Fri, Apr 4, 2014 at 8:34 AM, David Blaikie dblai...@gmail.com wrote: On Fri, Apr 4, 2014 at 8:24 AM, Lubos Lunak l.lu...@centrum.cz wrote: On Friday 04 of April 2014, Eric Christopher wrote: ... Why would you want to do this? The compile itself happens in a chroot and the expected and actual output locations differ (and don't even exist in the other tree). In your scenario the .dwo is not side-by-side with the .o? Or are you overriding the .o name as well in some way? The scenario is distributed compiling with Icecream (https://github.com/icecc/icecream). If you don't know it, think distcc, except more sophisticated in some ways, one of them being that it ships the compiler to the remote host and uses it in chroot (which avoids a number of problems and allows cross-compiling). Which means the actual compile run lacks a number of things, like the source file itself (piped using stdin), the source file location, or the expected output location. The result goes to a temporary .o file, which is generally not a problem (I don't think the name of the .o file matters), but with split dwarf the .dwo name gets hardcoded to this random location in the .o file. For performance reasons there's a chroot location per compiler, not per compile, so while I could temporarily create the right location, I'm not exactly eager to write the code to do that properly with all the locks etc. I'm not sure why that would require locks, etc. At least you could generate a safe temp directory and put the source file in that directory with its intended name - then you just have to correct for the directory, but not the file or dwo file names, right? ( -fdebug-compilation-dir is already there - but, yes, as pointed out it should be elevated to a driver option if it's being relied upon) Maybe this doesn't work for relative paths? (eg: clang x/y.cpp from directory 'z', but I don't recall how clang names the dwo file, the comp dir file, etc, in that case) when I could use a compiler option that just sets one dwarf info field, if only I actually could use it. If you want a precedent, there's already -fdebug-compilation-dir, which AFAICT is exactly the same, just with a different dwarf info field. I could do with changing DW_AT_GNU_dwo_name explicitly after the build, but that seems needlessly complex given that this seems to be exactly what the option does. I don't see why I would be allowed to override any option except for this one. -Xclang and the underlying driver arguments aren't really a stable/guaranteed interface. I'd be more inclined to accept this change if it were just for some debugging, but since it sounds like you want to rely on it, it's good for us to understand the goal and perhaps suggest or provide the best way of achieving it long-term. It's stable/guaranteed enough for me, and I'd rather have a clean solution that maybe breaks one day than something hackish the whole time. -- Lubos Lunak ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Allow overriding -split-dwarf-file
On Friday 02 of May 2014, David Blaikie wrote: On Fri, Apr 4, 2014 at 9:07 AM, Lubos Lunak l.lu...@centrum.cz wrote: On Friday 04 of April 2014, David Blaikie wrote: Oops, re-add cfe-commits. On Fri, Apr 4, 2014 at 8:34 AM, David Blaikie dblai...@gmail.com wrote: On Fri, Apr 4, 2014 at 8:24 AM, Lubos Lunak l.lu...@centrum.cz wrote: On Friday 04 of April 2014, Eric Christopher wrote: ... Why would you want to do this? The compile itself happens in a chroot and the expected and actual output locations differ (and don't even exist in the other tree). In your scenario the .dwo is not side-by-side with the .o? Or are you overriding the .o name as well in some way? The scenario is distributed compiling with Icecream (https://github.com/icecc/icecream). If you don't know it, think distcc, except more sophisticated in some ways, one of them being that it ships the compiler to the remote host and uses it in chroot (which avoids a number of problems and allows cross-compiling). Which means the actual compile run lacks a number of things, like the source file itself (piped using stdin), the source file location, or the expected output location. The result goes to a temporary .o file, which is generally not a problem (I don't think the name of the .o file matters), but with split dwarf the .dwo name gets hardcoded to this random location in the .o file. For performance reasons there's a chroot location per compiler, not per compile, so while I could temporarily create the right location, I'm not exactly eager to write the code to do that properly with all the locks etc. I'm not sure why that would require locks, etc. At least you could generate a safe temp directory and put the source file in that directory with its intended name - then you just have to correct for the directory, but not the file or dwo file names, right? ( -fdebug-compilation-dir is already there - but, yes, as pointed out it should be elevated to a driver option if it's being relied upon) Maybe this doesn't work for relative paths? (eg: clang x/y.cpp from directory 'z', but I don't recall how clang names the dwo file, the comp dir file, etc, in that case) It doesn't work for absolute paths. $ clang++ -c -g -gsplit-dwarf a.cpp -o /somewhere/a.o $ readelf -wi /somewhere/a.o | grep dwo_name c DW_AT_GNU_dwo_name: (indirect string, offset: 0x0): /somewhere/a.dwo Now how should I get the correct path for the dwo file, without locks and whatnot, if the same node gets two such compile jobs at the same time (not very likely, but possible) and would therefore need to output two different .o files with the same path and filename? -- Lubos Lunak ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Allow overriding -split-dwarf-file
On Fri, Apr 11, 2014 at 2:12 PM, Eric Christopher echri...@gmail.comwrote: On Fri, Apr 4, 2014 at 9:07 AM, Lubos Lunak l.lu...@centrum.cz wrote: -Xclang and the underlying driver arguments aren't really a stable/guaranteed interface. I'd be more inclined to accept this change if it were just for some debugging, but since it sounds like you want to rely on it, it's good for us to understand the goal and perhaps suggest or provide the best way of achieving it long-term. It's stable/guaranteed enough for me, and I'd rather have a clean solution that maybe breaks one day than something hackish the whole time. I'm guessing you mean that the actual output file name differs? I.e. some sort of temporary cached file ala ccache that doesn't have the same name as the file that you've just compiled? The directory ends up being the same here (or can be set with, as you surmised -fdebug-compilation-dir). If so, I don't mind an option to set here necessarily, but would like to see it plumbed through and not rely on Xclang options. (Reading email backlog...) Can we just bubble up a driver option for this? I bet we'd take a patch for that. ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Allow overriding -split-dwarf-file
On Fri, Apr 4, 2014 at 9:07 AM, Lubos Lunak l.lu...@centrum.cz wrote: On Friday 04 of April 2014, David Blaikie wrote: Oops, re-add cfe-commits. On Fri, Apr 4, 2014 at 8:34 AM, David Blaikie dblai...@gmail.com wrote: On Fri, Apr 4, 2014 at 8:24 AM, Lubos Lunak l.lu...@centrum.cz wrote: On Friday 04 of April 2014, Eric Christopher wrote: ... Why would you want to do this? The compile itself happens in a chroot and the expected and actual output locations differ (and don't even exist in the other tree). In your scenario the .dwo is not side-by-side with the .o? Or are you overriding the .o name as well in some way? The scenario is distributed compiling with Icecream (https://github.com/icecc/icecream). If you don't know it, think distcc, except more sophisticated in some ways, one of them being that it ships the compiler to the remote host and uses it in chroot (which avoids a number of problems and allows cross-compiling). Which means the actual compile run lacks a number of things, like the source file itself (piped using stdin), the source file location, or the expected output location. The result goes to a temporary .o file, which is generally not a problem (I don't think the name of the .o file matters), but with split dwarf the .dwo name gets hardcoded to this random location in the .o file. For performance reasons there's a chroot location per compiler, not per compile, so while I could temporarily create the right location, I'm not exactly eager to write the code to do that properly with all the locks etc. when I could use a compiler option that just sets one dwarf info field, if only I actually could use it. If you want a precedent, there's already -fdebug-compilation-dir, which AFAICT is exactly the same, just with a different dwarf info field. I could do with changing DW_AT_GNU_dwo_name explicitly after the build, but that seems needlessly complex given that this seems to be exactly what the option does. I don't see why I would be allowed to override any option except for this one. -Xclang and the underlying driver arguments aren't really a stable/guaranteed interface. I'd be more inclined to accept this change if it were just for some debugging, but since it sounds like you want to rely on it, it's good for us to understand the goal and perhaps suggest or provide the best way of achieving it long-term. It's stable/guaranteed enough for me, and I'd rather have a clean solution that maybe breaks one day than something hackish the whole time. I'm guessing you mean that the actual output file name differs? I.e. some sort of temporary cached file ala ccache that doesn't have the same name as the file that you've just compiled? The directory ends up being the same here (or can be set with, as you surmised -fdebug-compilation-dir). If so, I don't mind an option to set here necessarily, but would like to see it plumbed through and not rely on Xclang options. -eric ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Allow overriding -split-dwarf-file
... Why would you want to do this? -eric On Apr 4, 2014 12:26 AM, Lubos Lunak l.lu...@centrum.cz wrote: The option -split-dwarf-file is passed by the driver to the compiler after processing -Xclang options, thus overriding any possible explicitly specified option: $ clang++ -c -gsplit-dwarf a.cpp -o a.o -Xclang -split-dwarf-file -Xclang b.dwo $ readelf -wi a.o | grep dwo_name c DW_AT_GNU_dwo_name: (indirect string, offset: 0x0): a.dwo This is because the driver invokes the compiler as /usr/bin/clang-3.4 -cc1 ... -split-dwarf-file b.dwo -o a.o -x c++ a.cpp -split-dwarf-file a.dwo The attached patch fixes this. Ok to push? -- Lubos Lunak ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Allow overriding -split-dwarf-file
On Friday 04 of April 2014, Eric Christopher wrote: ... Why would you want to do this? The compile itself happens in a chroot and the expected and actual output locations differ (and don't even exist in the other tree). I could do with changing DW_AT_GNU_dwo_name explicitly after the build, but that seems needlessly complex given that this seems to be exactly what the option does. I don't see why I would be allowed to override any option except for this one. -eric On Apr 4, 2014 12:26 AM, Lubos Lunak l.lu...@centrum.cz wrote: The option -split-dwarf-file is passed by the driver to the compiler after processing -Xclang options, thus overriding any possible explicitly specified option: $ clang++ -c -gsplit-dwarf a.cpp -o a.o -Xclang -split-dwarf-file -Xclang b.dwo $ readelf -wi a.o | grep dwo_name c DW_AT_GNU_dwo_name: (indirect string, offset: 0x0): a.dwo This is because the driver invokes the compiler as /usr/bin/clang-3.4 -cc1 ... -split-dwarf-file b.dwo -o a.o -x c++ a.cpp -split-dwarf-file a.dwo The attached patch fixes this. Ok to push? -- Lubos Lunak ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Allow overriding -split-dwarf-file
Oops, re-add cfe-commits. On Fri, Apr 4, 2014 at 8:34 AM, David Blaikie dblai...@gmail.com wrote: On Fri, Apr 4, 2014 at 8:24 AM, Lubos Lunak l.lu...@centrum.cz wrote: On Friday 04 of April 2014, Eric Christopher wrote: ... Why would you want to do this? The compile itself happens in a chroot and the expected and actual output locations differ (and don't even exist in the other tree). In your scenario the .dwo is not side-by-side with the .o? Or are you overriding the .o name as well in some way? I could do with changing DW_AT_GNU_dwo_name explicitly after the build, but that seems needlessly complex given that this seems to be exactly what the option does. I don't see why I would be allowed to override any option except for this one. -Xclang and the underlying driver arguments aren't really a stable/guaranteed interface. I'd be more inclined to accept this change if it were just for some debugging, but since it sounds like you want to rely on it, it's good for us to understand the goal and perhaps suggest or provide the best way of achieving it long-term. -eric On Apr 4, 2014 12:26 AM, Lubos Lunak l.lu...@centrum.cz wrote: The option -split-dwarf-file is passed by the driver to the compiler after processing -Xclang options, thus overriding any possible explicitly specified option: $ clang++ -c -gsplit-dwarf a.cpp -o a.o -Xclang -split-dwarf-file -Xclang b.dwo $ readelf -wi a.o | grep dwo_name c DW_AT_GNU_dwo_name: (indirect string, offset: 0x0): a.dwo This is because the driver invokes the compiler as /usr/bin/clang-3.4 -cc1 ... -split-dwarf-file b.dwo -o a.o -x c++ a.cpp -split-dwarf-file a.dwo The attached patch fixes this. Ok to push? -- Lubos Lunak ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Allow overriding -split-dwarf-file
On Friday 04 of April 2014, David Blaikie wrote: Oops, re-add cfe-commits. On Fri, Apr 4, 2014 at 8:34 AM, David Blaikie dblai...@gmail.com wrote: On Fri, Apr 4, 2014 at 8:24 AM, Lubos Lunak l.lu...@centrum.cz wrote: On Friday 04 of April 2014, Eric Christopher wrote: ... Why would you want to do this? The compile itself happens in a chroot and the expected and actual output locations differ (and don't even exist in the other tree). In your scenario the .dwo is not side-by-side with the .o? Or are you overriding the .o name as well in some way? The scenario is distributed compiling with Icecream (https://github.com/icecc/icecream). If you don't know it, think distcc, except more sophisticated in some ways, one of them being that it ships the compiler to the remote host and uses it in chroot (which avoids a number of problems and allows cross-compiling). Which means the actual compile run lacks a number of things, like the source file itself (piped using stdin), the source file location, or the expected output location. The result goes to a temporary .o file, which is generally not a problem (I don't think the name of the .o file matters), but with split dwarf the .dwo name gets hardcoded to this random location in the .o file. For performance reasons there's a chroot location per compiler, not per compile, so while I could temporarily create the right location, I'm not exactly eager to write the code to do that properly with all the locks etc. when I could use a compiler option that just sets one dwarf info field, if only I actually could use it. If you want a precedent, there's already -fdebug-compilation-dir, which AFAICT is exactly the same, just with a different dwarf info field. I could do with changing DW_AT_GNU_dwo_name explicitly after the build, but that seems needlessly complex given that this seems to be exactly what the option does. I don't see why I would be allowed to override any option except for this one. -Xclang and the underlying driver arguments aren't really a stable/guaranteed interface. I'd be more inclined to accept this change if it were just for some debugging, but since it sounds like you want to rely on it, it's good for us to understand the goal and perhaps suggest or provide the best way of achieving it long-term. It's stable/guaranteed enough for me, and I'd rather have a clean solution that maybe breaks one day than something hackish the whole time. -- Lubos Lunak ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
