GNU Tools Cauldron 2019
Hi all, We are very pleased to invite you all to the GNU Tools Cauldron on 12-15 September 2019. This year's Cauldron will be held in Montreal, Canada. See the wiki page for details: https://gcc.gnu.org/wiki/cauldron2019 The conference is free to attend, registration in advance is required. As usual, please register, send abstracts and ask administrivia questions to tools-cauldron-admin AT googlegroups.com. The Cauldron is organized by a small ad hoc group of volunteers. We are keen to add some more people, so others can stand down. If you'd like to be part of that organizing committee, please email the same address. I have sent (or plan to send) this announcement to the main GCC, GDB, binutils, CGEN, DejaGnu, newlib and glibc mailing lists. Please feel free to share with any other groups that are appropriate. Looking forward to seeing you in Montreal next September! Simon
Re: GNU Tools Cauldron 2019
On 2019-03-14 5:28 p.m., Simon Marchi wrote: > Hi all, > > We are very pleased to invite you all to the GNU Tools Cauldron on 12-15 > September 2019. This year's Cauldron will be held in Montreal, Canada. > See the wiki page for details: > > https://gcc.gnu.org/wiki/cauldron2019 > > The conference is free to attend, registration in advance is required. As > usual, please register, send abstracts and ask administrivia > questions to tools-cauldron-admin AT googlegroups.com. > > The Cauldron is organized by a small ad hoc group of volunteers. We > are keen to add some more people, so others can stand down. If you'd > like to be part of that organizing committee, please email the same > address. > > I have sent (or plan to send) this announcement to the main GCC, GDB, > binutils, CGEN, DejaGnu, newlib and glibc mailing lists. Please feel free to > share with any other groups that are appropriate. > > Looking forward to seeing you in Montreal next September! > > Simon Hi again! This is a little reminder about the Cauldron 2019. If you plan on attending, please take a few minutes to send your registration (instructions are on the wiki [1]), it helps us greatly if you do it earlier than later. Also, we have received some very interesting talk and BoF submissions, but there is still room available. If you have worked on improving the GNU toolchain these past years, others are probably interested in hearing about it! If you would like to give a talk but don't have an abstract written yet, feel free to just send the title/topic for now and the abstract at a later time. We have room for full-length talks (~45 minutes plus questions), lightning talks (~10 minutes plus questions), BoFs as well as developer tutorials. Simon [1] https://gcc.gnu.org/wiki/cauldron2019
Re: GNU Tools Cauldron 2019
On 2019-07-25 3:13 p.m., Simon Marchi wrote: > Hi again! > > This is a little reminder about the Cauldron 2019. If you plan on attending, > please > take a few minutes to send your registration (instructions are on the wiki > [1]), it > helps us greatly if you do it earlier than later. > > Also, we have received some very interesting talk and BoF submissions, but > there is > still room available. If you have worked on improving the GNU toolchain > these past > years, others are probably interested in hearing about it! If you would like > to > give a talk but don't have an abstract written yet, feel free to just send the > title/topic for now and the abstract at a later time. We have room for > full-length > talks (~45 minutes plus questions), lightning talks (~10 minutes plus > questions), > BoFs as well as developer tutorials. > > Simon > > [1] https://gcc.gnu.org/wiki/cauldron2019 Hi all, The list of talks and BoFs for the GNU Tools Cauldron 2019 has now been published, you can consult it at: https://gcc.gnu.org/wiki/cauldron2019#Abstracts Please note: - If you have sent a registration email but did not receive a confirmation, it might have fallen through the cracks, please send it again. - Same thing if you have sent a talk/BoF proposal and have not heard back, please send it again. - If you have received a confirmation for your talk/BoF, but don’t see it in the list above, it’s an error, please notify me. - If you notice a typo, a formatting problem, or would like to update the title/abstract of your talk, don’t hesitate to notify me. If you would like to attend but haven’t registered yet, it is still time to do so. Just follow the instructions here: https://gcc.gnu.org/wiki/cauldron2019#Registration In theory, this is the last time I spam the public lists about the Cauldron (for this year!), further details about the logistics will be sent prior to the event to those who registered. Thank you, Simon
Re: gdb 8.x - g++ 7.x compatibility
On 2018-02-02 22:17, Roman Popov wrote:
Hello,
I'm trying to switch from g++ 5.4 to g++ 7.2.
GDB 8.0.1 however does not understand RTTI generated by g++7.2, so my
Python scripts for GDB are not working.
Here is a code example:
struct base { virtual ~base(){} };
template< int IVAL, unsigned UVAL, unsigned long long ULLVAL>
struct derived : base {
int x = IVAL + + UVAL + ULLVAL;
};
int main()
{
base * o = new derived<1,2,3>{};
return 0;
}
When compiled with g++5.4 I can read value of x in debugger.
When compiled with g++7.2 gdb reports:
warning: RTTI symbol not found for class 'derived<1, 2u, 3ull>'
Problem here is that type name saved in debug information is
*derived<1, 2, 3>*, not *derived<1, 2u, 3ull>*
Do you plan to fix this anytime soon?
Thanks,
Roman
Hi Roman,
Your problem is probably linked to these issues, if you want to follow
them:
gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81932
gdb: https://sourceware.org/bugzilla/show_bug.cgi?id=22013
As Carl said, it's a good idea to try with the latest version of both
tools, but I think the issue will still be present.
GCC changed how it outputs unsigned template parameters in the debug
info (from 2u to just 2), and it doesn't look like it's going to change
it back. So I suppose we'll have to find a way to make GDB deal with
it.
Simon
Re: gdb 8.x - g++ 7.x compatibility
On 2018-02-03 13:35, Manfred wrote: > n4659 17.4 (Type equivalence) p1.3: > > Two template-ids refer to the same class, function, or variable if > ... > their corresponding non-type template arguments of integral or > enumeration type have identical values > ... > > It looks that for non-type template arguments the template type > equivalence is based on argument /value/ not /type/ (and value), so > IMHO gcc is correct where it considers foo<10u> and foo<10> to be the > same type, i.e. "refer to the same class" > > FWIW, type_info reports the same class name for both templates, which > appears to be correct as per the above. > > I would think someone from gcc might be more specific on why both > templates print 4294967286, and what debug info is actually stored by > -g in this case. I think that Roman's example clearly shows that they are not equivalent in all cases. Building Roman's example with g++ 7.3 results in a single instantiated type. You can see that both "new foo<10>()" and "new foo<10u>()" end up calling the same constructor. It seems like which type is instantiated depends on which template parameter (the signed or unsigned one) you use first. So with this: base * fi = new foo<10>(); base * fu = new foo<10u>(); the output is -10 for both, and with base * fu = new foo<10u>(); base * fi = new foo<10>(); the output is 4294967286 for both. But it's probably a bogus behavior. I tested with clangd, it instantiates two different types, so you get 4294967286 for the <10u> case and -10 for the <10> case. I also just built gcc from master, and it also instantiates two types, so it seems like that was fixed recently. So let's see what debug info gcc master generates for these two instances of foo (clang master generates the equivalent). <1><9257>: Abbrev Number: 66 (DW_TAG_structure_type) <9258> DW_AT_name: (indirect string, offset: 0x8455): foo<10> <925c> DW_AT_byte_size : 16 <925d> DW_AT_decl_file : 1 <925e> DW_AT_decl_line : 7 <925f> DW_AT_decl_column : 8 <9260> DW_AT_containing_type: <0x92fd> <9264> DW_AT_sibling : <0x92f8> ... <1><93be>: Abbrev Number: 66 (DW_TAG_structure_type) <93bf> DW_AT_name: (indirect string, offset: 0x8455): foo<10> <93c3> DW_AT_byte_size : 16 <93c4> DW_AT_decl_file : 1 <93c5> DW_AT_decl_line : 7 <93c6> DW_AT_decl_column : 8 <93c7> DW_AT_containing_type: <0x92fd> <93cb> DW_AT_sibling : <0x945f> If there are two types with the same name, how is gdb expected to differentiate them? If we can't rely on the DW_AT_name anymore to differentiate templated types, then the only alternative I see would be to make GDB ignore the template part of the DW_AT_name value, and reconstruct it in the format it expects (with the u) from the DW_TAG_template_value_param DIEs children of DW_TAG_structure_type (there's already code to do that in dwarf2_compute_name). Their types correctly point to the signed int or unsigned int DIE, so we have the necessary information. However, that would mean reading many more full DIEs early on, when just building partial symbols, which would slow done loading the symbols of pretty much any C++ program. >From what I understand from the original change that caused all this [1], >removing the suffixes was meant to make the error messages more readable for the user. However, since foo<10>::print() and foo<10u>::print() are not the same function, I think it would actually be more confusing if an error message talked about the instantiation with the unsigned type, but mentioned "foo<10>::print()". For example, if you put a static_assert (std::is_signed::value); in the print method, this is the error message from gcc: test.cpp: In instantiation of 'void foo::print() [with auto IVAL = 10]': test.cpp:24:1: required from here test.cpp:12:22: error: static assertion failed static_assert (std::is_signed::value); ^~~ Wouldn't the message make more sense with a u suffix? Simon [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78165
Re: gdb 8.x - g++ 7.x compatibility
Hi Martin,
Thanks for the reply.
On 2018-02-04 02:17 PM, Martin Sebor wrote:
> Printing the suffix is unhelpful because it leads to unnecessary
> differences in diagnostics (even in non-template contexts). For
> templates with non-type template parameters there is no difference
> between, say A<1>, A<1U>, A<(unsigned) 1>, or even A when
> Green is an enumerator that evaluates to 1, so including the suffix
> serves no useful purpose.
This is the part I don't understand. In Roman's example, spelling
foo<10> and foo<10u> resulted in two different instantiations of the
template, with different code. So that means it can make a difference,
can't it?
> In the GCC test suite, it would tend to
> cause failures due to differences between the underlying type of
> common typedefs like size_t and ptrdiff_t. Avoiding these
> unnecessary differences was the main motivation for the change.
> Not necessarily just in the GCC test suite but in all setups that
> process GCC messages.
Ok, I understand.
> I didn't consider the use of auto as a template parameter but
> I don't think it changes anything. There, just like in other
> contexts, what's important is the deduced types and the values
> of constants, not the minute details of how they are spelled.
Well, it seems like using decltype on a template constant value is
a way to make the type of constants important, in addition to their
value. I know the standard seems to say otherwise (what Manfred
quoted), but the reality seems different. I'm not a language expert
so I can't tell if this is a deficiency in the language or not.
> That said, it wasn't my intention to make things difficult for
> the debugger.
I hope so :).
> But changing GCC back to include the suffix,
> even just in the debug info, isn't a solution. There are other
> compilers besides GCC that don't emit the suffixes, and there
> even are some that prepend a cast to the number, so if GDB is
> to be usable with all these kinds of producers it needs to be
> able to handle all of these forms.
As I said earlier, there are probably ways to make GDB cope with it.
The only solution I saw (I'd like to hear about other ones) was to make
GDB ignore the template part in DW_AT_name and re-build it from the
DW_TAG_template_* DIEs in the format it expects. It can already do
that somewhat, because, as you said, some compilers don't emit
the template part in DW_AT_name.
Doing so would cause major slowdowns in symbol reading, I've tried it
for the sake of experimentation/discussion. I have a patch available
on the "users/simark/template-suffix" branch in the binutils-gdb
repo [1]. It works for Roman's example, but running the GDB testsuite
shows that, of course, the devil is in the details.
Consider something like this:
template
struct foo { virtual ~foo() {} };
int n;
int main ()
{
foo<&n> f;
}
The demangled name that GDB will be looking up is "foo<&n>". The
debug info about the template parameter only contains the resulting
address of n (the value of &n):
<2>: Abbrev Number: 11 (DW_TAG_template_value_param)
DW_AT_name: P
DW_AT_type: <0x1ac>
DW_AT_location: 10 byte block: 3 34 10 60 0 0 0 0 0 9f
(DW_OP_addr: 601034; DW_OP_stack_value)
I don't see how GDB could reconstruct the "&n" in the template, so
that's where my idea falls short.
Simon
[1]
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=shortlog;h=refs/heads/users/simark/template-suffix
Re: gdb 8.x - g++ 7.x compatibility
On 2018-02-05 11:45, Martin Sebor wrote: Yes, with auto, the type of the constant does determine the type of the specialization of the template in the source code. In non-type template arguments, and more to the point I was making, in diagnostics, the suffix shouldn't or doesn't need to be what distinguishes the type of the template, even with auto. The part "with auto IVAL = 10" in the message 'void foo::print() [with auto IVAL = 10]': would be far clearer if auto were replaced by the deduced type, say along these lines: 'void foo::print() [with int IVAL = 10]': rather than relying on the suffix alone to distinguish between different specializations of the template. That seems far too subtle to me. But I think the diagnostic format is (or should be) independent of the debug info. That makes sense. With respect to the suffix, I keep coming back to the reality that even if GCC were to change to emit a format that GDB can interpret easily and efficiently, there still are other compilers that emit a different format. So the conclusion that a general solution that handles more than just one format (at least for non-type template arguments without auto) seems unescapable. If there are other compilers we wanted to support for which we can't trust the template format, we could always ignore the template part of DW_AT_name specifically for them. But since g++ and gdb are part of the same project and are expected to work well and efficiently together, I would have hoped that we could agree on a format so that gdb would not have to do the extra work when parsing a g++-generated file (consequently the same format that libiberty's demangler produces). Given the problem I illustrated in my previous mail, I don't have a general solution to the problem to propose. Simon
Re: gdb 8.x - g++ 7.x compatibility
On 2018-02-07 02:21, Daniel Berlin wrote: As the person who, eons ago, wrote a bunch of the the GDB code for this C++ ABI support, and as someone who helped with DWARF support in both GDB and GCC, let me try to propose a useful path forward (in the hopes that someone will say "that's horrible, do it this instead") Here are the constraints i believe we are working with. 1. GDB should work with multiple DWARF producers and multiple C++ compilers implementing the C++ ABI 2. There is no canonical demangled format for the C++ ABI 3. There is no canoncial target demangler you can say everyone should use (and even if there was, you don't want to avoid debugging working because someone chose not to) 4. You don't want to slow down GDB if you can avoid it 5. Despite them all implementation the same ABI, it's still possible to distinguish the producers by the producer/compiler in the dwarf info. Given all that: GDB has ABI hooks that tell it what to do for various C++ ABIs. This is how it knows to call the right demangler for gcc v3's abi vs gcc v2's abi. and handle various differences between them. See gdb/cp-abi.h The IMHO, obvious thing to do here is: Handle the resulting demangler differences with 1 or more new C++ ABI hooks. Or, introduce C++ debuginfo producer hooks that the C++ ABI hooks use if folks want it to be separate. Once the producer is detected, fill in the hooks with a set of functions that does the right thing. I imagine this would also clean up a bundle of hacks in various parts of gdb trying to handle these differences anyway (which is where a lot of the multiple symbol lookups/etc that are often slow come from. If we just detected and said "this is gcc 6, it behaves like this", we wouldn't need to do that) In case you are worried, you will discover this is how a bunch of stuff is done and already contains a ball of hacks. Using hooks would be, IMHO, a significant improvement. Hi Daniel, Thanks for chiming in. This addresses the issue of how to do good software design in GDB to support different producers cleanly, but I think we have some issues even before that, like how to support g++ 7.3 and up. I'll try to summarize the issue quickly. It's now possible to end up with two templated classes with the same name that differ only by the signedness of their non-type template parameter. One is Foo and the other is Foo (the 10 is unsigned). Until 7.3, g++ would generate names like Foo<10> for the former and names like Foo<10u> for the later (in the DW_AT_name attribute of the classes' DIEs). Since 7.3, it produces Foo<10> for both. When GDB wants to know the run time type of an object, it fetches the pointer to its vtable, does a symbol lookup to get the linkage name and demangles it, which gives a string like "vtable for Foo<10>" or "vtable for Foo<10u>". It strips the "vtable for " and uses the remainder to do a type lookup. Since g++ 7.3, you can see that doing a type lookup for Foo<10> may find the wrong type, and doing a lookup for Foo<10u> won't find anything. So the problem here is how to uniquely identify those two classes when we are doing this run-time type finding operation (and probably in other cases too). Simon
Re: gdb 8.x - g++ 7.x compatibility
On 2018-02-07 11:26, Michael Matz wrote: Hi, On Wed, 7 Feb 2018, Simon Marchi wrote: This addresses the issue of how to do good software design in GDB to support different producers cleanly, but I think we have some issues even before that, like how to support g++ 7.3 and up. I'll try to summarize the issue quickly. It's now possible to end up with two templated classes with the same name that differ only by the signedness of their non-type template parameter. One is Foo and the other is Foo (the 10 is unsigned). Until 7.3, g++ would generate names like Foo<10> for the former and names like Foo<10u> for the later (in the DW_AT_name attribute of the classes' DIEs). Since 7.3, it produces Foo<10> for both. Yeah, gdb needs a way to lookup types by name, and since the change DW_AT_name can't be used for this anymore. Either that needs to be fixed/reverted, or we do the more obvious thing: since types in C++ have linkage it makes sense to add the linkage (i.e. mangled) name to the types DIE using the traditional DW_AT_MIPS_linkage_name. That latter solution would have the advantage that you don't need to demangle anything anymore. From vtable you get to typeinfo, from there for typeinfo name, and that contains the mangled type name (without _Z prefix). But do struct/classes have mangled names? Simon
Re: gdb 8.x - g++ 7.x compatibility
On 2018-02-07 11:50, Jonathan Wakely wrote: On 7 February 2018 at 16:36, Simon Marchi wrote: On 2018-02-07 11:26, Michael Matz wrote: Hi, On Wed, 7 Feb 2018, Simon Marchi wrote: This addresses the issue of how to do good software design in GDB to support different producers cleanly, but I think we have some issues even before that, like how to support g++ 7.3 and up. I'll try to summarize the issue quickly. It's now possible to end up with two templated classes with the same name that differ only by the signedness of their non-type template parameter. One is Foo and the other is Foo (the 10 is unsigned). Until 7.3, g++ would generate names like Foo<10> for the former and names like Foo<10u> for the later (in the DW_AT_name attribute of the classes' DIEs). Since 7.3, it produces Foo<10> for both. Yeah, gdb needs a way to lookup types by name, and since the change DW_AT_name can't be used for this anymore. Either that needs to be fixed/reverted, or we do the more obvious thing: since types in C++ have linkage it makes sense to add the linkage (i.e. mangled) name to the types DIE using the traditional DW_AT_MIPS_linkage_name. That latter solution would have the advantage that you don't need to demangle anything anymore. From vtable you get to typeinfo, from there for typeinfo name, and that contains the mangled type name (without _Z prefix). But do struct/classes have mangled names? Yes. Interesting. What do they look like, and in what context do they appear? Simon
Re: gdb 8.x - g++ 7.x compatibility
On 2018-02-07 12:08, Jonathan Wakely wrote:
Why would they not have a mangled name?
Interesting. What do they look like, and in what context do they
appear?
Anywhere you need a name for linkage purposes, such as in a function
signature, or as a template argument of another type, or in the
std::type_info::name() for the type etc. etc.
$ g++ -o test.o -c -x c++ - <<< 'struct X {}; void f(X) {}
template struct Y { }; void g(Y) {}' && nm
--defined-only test.o
T _Z1f1X
0007 T _Z1g1YI1XE
The mangled name for X is "X" and the mangled name for Y is "YI1XE"
which includes the name "X".
This isn't really on-topic for solving the GDB type lookup problem
though.
Ah ok, the class name appears mangled in other entities' mangled name.
But from what I understand there's no mangled name for the class such
that
echo | c++filt
outputs the class name (e.g. "Foo<10>"). That wouldn't make sense,
since there's no symbol for the class itself.
Simon
Re: gdb 8.x - g++ 7.x compatibility
On 2018-02-07 12:30, Jonathan Wakely wrote: Ah ok, the class name appears mangled in other entities' mangled name. But from what I understand there's no mangled name for the class such that echo | c++filt outputs the class name (e.g. "Foo<10>"). That wouldn't make sense, since there's no symbol for the class itself. echo _Z3FooILi10EE | c++filt Ok, thanks for the precision!
Re: Split DWARF and rnglists, gcc vs clang
On 2020-11-12 7:11 p.m., Mark Wielaard wrote: > Hi Simon, > > On Thu, Nov 05, 2020 at 11:11:43PM -0500, Simon Marchi wrote: >> I'm currently squashing some bugs related to .debug_rnglists in GDB, and >> I happened to notice that clang and gcc do different things when >> generating rnglists with split DWARF. I'd like to know if the two >> behaviors are acceptable, and therefore if we need to make GDB accept >> both. Or maybe one of them is not doing things correctly and would need >> to be fixed. >> >> clang generates a .debug_rnglists.dwo section in the .dwo file. Any >> DW_FORM_rnglistx attribute in the DWO refers to that section. That >> section is not shared with any other object, so DW_AT_rnglists_base is >> never involved for these attributes. Note that there might still be a >> DW_AT_rnglists_base on the DW_TAG_skeleton_unit, in the linked file, >> used if the skeleton itself has an attribute of form DW_FORM_rnglistx. >> This rnglist would be found in a .debug_rnglists section in the linked >> file, shared with the other units of the linked file. >> >> gcc generates a single .debug_rnglists in the linked file and no >> .debug_rnglists.dwo in the DWO files. So when an attribute has form >> DW_FORM_rnglistx in a DWO file, I presume we need to do the lookup in >> the .debug_rnglists section in the linked file, using the >> DW_AT_rnglists_base attribute found in the corresponding skeleton unit. >> This looks vaguely similar to how it was done pre-DWARF 5, with >> DW_AT_GNU_ranges base. >> >> So, is gcc wrong here? I don't see anything in the DWARF 5 spec >> prohibiting to do it like gcc does, but clang's way of doing it sounds >> more in-line with the intent of what's described in the DWARF 5 spec. >> So I wonder if it's maybe an oversight or a misunderstanding between the >> two compilers. > > I think I would have asked the question the other way around :) The > spec explicitly describes rnglists_base (and loclists_base) as a way > to reference ranges (loclists) through the index table, so that the > only relocation you need is in the (skeleton) DIE. I presume you reference this non-normative text in section 2.17.3? This range list representation, the rnglist class, and the related DW_AT_rnglists_base attribute are new in DWARF Version 5. Together they eliminate most or all of the object language relocations previously needed for range lists. What I understand from this is that the rnglist class and DW_AT_rnglists_base attribute help reduce the number of relocations in the non-split case (it removes the need for relocations from DW_AT_ranges attribute values in .debug_info to .debug_rnglists). I don't understand it as saying anything about where to put the rnglist data in the split-unit case. > But the rnglists > (loclists) themselves can still use relocations. A large part of them > is non-shared addresses, so using indexes (into the .debug_addr > addr_base) would simply be extra overhead. The relocations will > disappear once linked, but the index tables won't. > > As an alternative, if you like to minimize the amount of debug data in > the main object file, the spec also describes how to put a whole > .debug_rnglists.dwo (or .debug_loclists.dwo) in the split dwarf > file. Then you cannot use all entry encodings and do need to use an > .debug_addr index to refer to any addresses in that case. So the > relocations are still there, you just refer to them through an extra > index indirection. > > So I believe both encodings are valid according to the spec. It just > depends on what you are optimizing for, small main object file size or > smallest encoding with least number of indirections. So, if I understand correctly, gcc's way of doing things (putting all the rnglists in a common .debug_rnglists section) reduces the overall size of debug info since the rnglists can use the direct addressing rnglists entries (e.g. DW_RLE_start_end) rather than the indirect ones (e.g. DW_RLE_startx_endx). But this come at the expense of a lot of relocations in the rnglists themselves, since they refer to addresses directly. I thought that the main point of split-units was to reduce the number of relocations processed by the linker and data moved around by the linker, to reduce link time and provide a better edit-build-debug cycle. Is that the case? Anyway, regardless of the intent, the spec should ideally be clear about that so we don't have to guess. > P.S. I am really interested in these interpretations of DWARF, but I > don't really follow the gdb implementation details very much. Could we > maybe move discussions like these from the -patches list to the main > gdb (or gcc) mailinglist? Sure, I added gdb@ and gcc@. I also left gdb-patches@ so that it's possible to follow the discussion there. Simon
Re: Split DWARF and rnglists, gcc vs clang
On 2020-11-13 10:18 a.m., Mark Wielaard wrote: > That too, but I was actually referring to the sections that define > Range List and Location List Tables (7.28 and 7.29) which define the > meaning of DW_AT_rnglists_base and DW_AT_loclists_base. But you could > also look at 3.1.3 Split Full Compilation Unit Entries which says that > those base attributes are inherited from the corresponding skeleton > compilation unit for a split unit. Hmm, indeed, if we interpret that sentence in 3.1.3 to the letter, it suggests that the the DW_FORM_rnglistx attributes in the DWO are meant to point in the linked file's .debug_rnglists. Otherwise, inheriting DW_AT_rnglists_base wouldn't be meaningful. But when DWO files use a .debug_rnglists.dwo section, it doesn't make sense to consider the inherited DW_AT_rnglists_base. So in the end the logical thing to do when encountering a DW_FORM_rnglistx in a split-unit, in order to support everybody, is probably to go to the .debug_rnglists.dwo section, if there's one, disregarding the (inherited) DW_AT_rnglists_base. If there isn't, then try the linked file's .debug_rnglists section, using DW_AT_rnglists_base. If there isn't, then something is malformed. >> What I understand from this is that the rnglist class and >> DW_AT_rnglists_base attribute help reduce the number of relocations in >> the non-split case (it removes the need for relocations from >> DW_AT_ranges attribute values in .debug_info to .debug_rnglists). I >> don't understand it as saying anything about where to put the rnglist >> data in the split-unit case. > > I interpreted it as when there is a base attribute in the (skeleton) > unit, then the corresponding section (index table) can be found in the > main object file. That doesn't work with how clang produces it, AFAIU. There is a DW_AT_rnglists_base attribute in the skeleton and a .debug_rnglists in the linked file, which is used for the skeleton's DW_AT_ranges attribute. And there is also .debug_rnglists.dwo sections in the DWO files. So DW_FORM_rnglistx values in the skeleton use the .debug_rnglists in the linked file, while the DW_FORM_rnglistx values in the DWO file use the .debug_rnglists.dwo in that file (even though there is a DW_AT_rnglists_base in the skeleton). > At least that is how elfutils libdw interprets the > base attributes, not just for rnglists_base, but also str_offsets_base, > addr_base, etc. And that is also how/when GCC emits them. > >>> So I believe both encodings are valid according to the spec. It just >>> depends on what you are optimizing for, small main object file size or >>> smallest encoding with least number of indirections. >> >> So, if I understand correctly, gcc's way of doing things (putting all >> the rnglists in a common .debug_rnglists section) reduces the overall >> size of debug info since the rnglists can use the direct addressing >> rnglists entries (e.g. DW_RLE_start_end) rather than the indirect ones >> (e.g. DW_RLE_startx_endx). But this come at the expense of a lot of >> relocations in the rnglists themselves, since they refer to addresses >> directly. > > Yes, and it reduces the number of .debug_addr entries that need > relocations. > >> I thought that the main point of split-units was to reduce the number of >> relocations processed by the linker and data moved around by the linker, >> to reduce link time and provide a better edit-build-debug cycle. Is >> that the case? > > I think it depends on who exactly you ask and what their specific > goals/setups are. Both things, reducing the number of relocations and > moving data out of the main object file, are independently useful in > different context. But I think it is mainly reducing the number of > relocations that is beneficial. For example clang (but not yet gcc) > supports having the .dwo sections themselves in the main object file > (using SHF_EXCLUDED for the .dwo sections, so the linker will still > skip them). Which is also a possibility that the spec describes and > which really makes split DWARF much more usable, because then you don't > need to change your build system to deal with multiple output files. Not sure I understand. Does that mean that the .dwo sections are emitted in the .o files, and that's the end of the road for them? The DW_AT_dwo_name attributes of the skeletons then refer to the .o files? Simon
Default debug format for AVR
Hi, The default debug format (when using only -g) for the AVR target is stabs. Is there a reason for it not being DWARF, and would it be possible to maybe consider possibly thinking about making it default to DWARF? I am asking because the support for stabs in GDB is pretty much untested and bit-rotting, so I think it would be more useful for everyone to use DWARF. Simon
Re: Default debug format for AVR
On 2021-04-05 3:36 p.m., Jim Wilson wrote:> On Sat, Apr 3, 2021 at 6:24 PM Simon Marchi via Gcc mailto:gcc@gcc.gnu.org>> wrote: > > The default debug format (when using only -g) for the AVR target is > stabs. Is there a reason for it not being DWARF, and would it be > possible to maybe consider possibly thinking about making it default to > DWARF? I am asking because the support for stabs in GDB is pretty much > untested and bit-rotting, so I think it would be more useful for > everyone to use DWARF. > > > I tried to deprecate the stabs support a little over 4 years ago. > https://gcc.gnu.org/pipermail/gcc-patches/2017-December/489296.html > <https://gcc.gnu.org/pipermail/gcc-patches/2017-December/489296.html> > There was a suggestion to change the error to a warning, but my startup > company job kept me so busy I never had a chance to follow up on this. > > I would like to see the stabs support deprecated and the later removed from > gcc. No new features have been added in a long time, and it is only being > maintained in the sense that when it fails it is fixed to ignore source code > constructs that it doesn't support. The longer it survives in this state, > the less useful it becomes. > > Jim You have 100% my suppose on this. The longer stabs survives (especially as the default for an arch), the longer some people who don't know the intricacies of debug formats could use it without knowing, and that does them a disservice. Simon
Re: Default debug format for AVR
On 2021-04-08 9:11 a.m., David Edelsohn wrote: >>> AIX continues to use and support STABS, although it is transitioning >>> to DWARF. If this is intended as a general statement about removal of >>> STABS support in GCC, >> >> Yes, it is. >> >> Richard. > > Richard, > > It is inappropriate to unilaterally make this decision without > discussion with all affected ports and maintainers, without warning, > and without deprecation. I request that you rescind this decision. > > It is somewhat ironic to act as a dictator when we are having a > discussion about dictatorial behavior in GCC leadership. I don't really want to start such a debate about GCC politics. If stabs is not ready to be deleted, that's fine. But it would be good to go through all targets for which it is the default (like avr), and see if they are ready to be switched to DWARF. That's a baby step towards eventually deleting it. Simon
Re: [RFC] add regenerate Makefile target
On 2024-03-13 04:02, Christophe Lyon via Gdb wrote: > Hi! > > After recent discussions on IRC and on the lists about maintainer-mode > and various problems with auto-generated source files, I've written > this small prototype. > > Based on those discussions, I assumed that people generally want to > update autotools files using a script similar to autoregen.py, which > takes care of running aclocal, autoheader, automake and autoconf as > appropriate. > > What is currently missing is a "simple" way of regenerating other > files, which happens normally with --enable-maintainer-mode (which is > reportedly broken). This patch as a "regenerate" Makefile target > which can be called to update those files, provided > --enable-maintainer-mode is used. > > I tried this approach with the following workflow for binutils/gdb: > - run autoregen.py in srcdir > - cd builddir > - configure --enable-maintainer-mode > - make all-bfd all-libiberty regenerate -j1 > - for gdb: make all -C gdb/data-directory -j1 > - make all -jXXX > > Making 'all' in bfd and libiberty is needed by some XXX-gen host > programs in opcodes. > > The advantage (for instance for CI) is that we can regenerate files at > -j1, thus avoiding the existing race conditions, and build the rest > with -j XXX. > > Among drawbacks: > - most sub-components use Makefile.am, but gdb does not: this may make > maintenance more complex (different rules for different projects) > - maintaining such ad-hoc "regenerate" rules would require special > attention from maintainers/reviewers > - dependency on -all-bfd and all-libiberty is probably not fully >intuitive, but should not be a problem if the "regenerate" rules >are used after a full build for instance > > Of course Makefile.def/Makefile.tpl would need further cleanup as I > didn't try to take gcc into account is this patch. > > Thoughts? My first thought it: why is it a Makefile target, instead of some script on the side (like autoregen.sh). It would be nice / useful to be able to it without configuring / building anything. For instance, the autoregen buildbot job could run it without configuring anything. Ideally, the buildbot wouldn't maintain its own autoregen.py file on the side, it would just use whatever is in the repo. Looking at the rule to re-generate copying.c in gdb for instance: # Make copying.c from COPYING $(srcdir)/copying.c: @MAINTAINER_MODE_TRUE@ $(srcdir)/../COPYING3 $(srcdir)/copying.awk awk -f $(srcdir)/copying.awk \ < $(srcdir)/../COPYING3 > $(srcdir)/copying.tmp mv $(srcdir)/copying.tmp $(srcdir)/copying.c There is nothing in this code that requires having configured the source tree. This code could for instance be moved to some generate-copying-c.sh script. generate-copying-c.sh could be called by an hypothetical autoregen.sh script, as well as the copying.c Makefile target, if we want to continue supporting the maintainer mode. Much like your regenerate targets, an autoregen.sh script in a given directory would be responsible to re-generate all the files in this directory that are generated and checked in git. It would also be responsible to call any autoregen.sh file in subdirectories. There's just the issue of files that are generated using tools that are compiled. When experimenting with maintainer mode the other day, I stumbled on the opcodes/i386-gen, for instance. I don't have a good solution to that, except to rewrite these tools in a scripting language like Python. Simon
Re: [RFC] add regenerate Makefile target
On 2024-03-15 04:50, Christophe Lyon via Gdb wrote: > On Thu, 14 Mar 2024 at 19:10, Simon Marchi wrote: >> My first thought it: why is it a Makefile target, instead of some script >> on the side (like autoregen.sh). It would be nice / useful to be >> able to it without configuring / building anything. For instance, the >> autoregen buildbot job could run it without configuring anything. >> Ideally, the buildbot wouldn't maintain its own autoregen.py file on the >> side, it would just use whatever is in the repo. > > Firstly because of what you mention later: some regeneration steps > require building host tools first, like the XXX-gen in opcodes. "build" and not "host", I think? > Since the existing Makefiles already contain the rules to autoregen > all these files, it seemed natural to me to reuse them, to avoid > reinventing the wheel with the risk of introducing new bugs. I understand. Although one advantage of moving the actual code out of the Makefile (even if there's still a Makefile rule calling the external script), is that it's much easier to maintain. Editors are much more useful when editing a standalone shell script than editing shell code in a Makefile target. It doesn't have to be this big one liner if you want to use variables, you don't need to escape $, you can run it through linters, you can call it by hand, etc. This is what I did here, for instance: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=f39632d9579d3c97f1e50a728efed3c5409747d2 So I think there's value in any case of moving the regeneration logic out of the Makefiles per se. > This involves changes in places where I've never looked at before, so > I'd rather reuse as much existing support as possible. > > For instance, there are the generators in opcodes/, but also things in > sim/, bfd/, updates to the docs and potfiles. In gcc, there's also > something "unusual" in fixincludes/ and libgfortran/ > > In fact, I considered also including 'configure', 'Makefile.in', > etc... in the 'regenerate' target, it does not seem natural to me to > invoke a script on the side, where you have to replicate the behaviour > of existing Makefiles, possibly getting out-of-sync when someone > forgets to update either Makefile or autoregen.py. I'm not sure I follow. Are you referring to the rules that automake automatically puts to re-generate Makefile.in and others when Makefile.am has changed? Your regenerate target would depend on those builtin rules? Let's say my generate-autostuff.sh script does: aclocal --some-flags automake --some-other-flags autoconf --some-other-other-flags And the checked-in Makefile.in is regenerated based on that. Wouldn't the built-in rules just call aclocal/automake/autoconf with those same flags? I don't see why they would get out of sync. > What is currently > missing is a way to easily regenerate files without having to run a > full 'make all' (which currently takes care of calling autoconf & > friends to update configure/Makefile.in). > > But yeah, having to configure before being able to regenerate files is > a bit awkward too :-) I understand the constraints your are working with, and I guess that doing: ./configure && make regenerate is not too bad. The buildbot could probably do that... except that it would need a way to force regenerate everything, ignoring the timestamps. Perhaps this option of GNU make would work? -B, --always-make Unconditionally make all targets. >> Looking at the rule to re-generate copying.c in gdb for instance: >> >> # Make copying.c from COPYING >> $(srcdir)/copying.c: @MAINTAINER_MODE_TRUE@ $(srcdir)/../COPYING3 >> $(srcdir)/copying.awk >>awk -f $(srcdir)/copying.awk \ >>< $(srcdir)/../COPYING3 > $(srcdir)/copying.tmp >>mv $(srcdir)/copying.tmp $(srcdir)/copying.c >> >> There is nothing in this code that requires having configured the source >> tree. This code could for instance be moved to some >> generate-copying-c.sh script. generate-copying-c.sh could be called by >> an hypothetical autoregen.sh script, as well as the copying.c Makefile >> target, if we want to continue supporting the maintainer mode. > Wouldn't it be more obscure than now? Currently such build rules are > all in the relevant Makefile. You'd have to open several scripts to > discover what's involved with updating copying.c Maybe, that's subjective :). The logic to regenerate would be in one script, and yes that script could be invoked from different places. At least the paper trail would be relatively easy to fol
Re: [RFC] add regenerate Makefile target
On 3/18/24 13:28, Christophe Lyon via Gdb wrote: > I'm not up-to-date with gdb's policy about patches: are they supposed > to be posted with or without the regenerated parts included? > IIUC they are not included in patch submissions for binutils and gcc, > which makes the pre-commit CI miss some patches. We post the patches with the regenerated parts. Simon
Re: [RFC] add regenerate Makefile target
On 3/18/24 13:25, Christophe Lyon wrote: > Well the rule to regenerate Makefile.in (eg in in opcodes/) is a bit > more complex > than just calling automake. IIUC it calls automake --foreign it any of > *.m4 file from $(am__configure_deps) that is newer than Makefile.in > (with an early exit in the loop), does nothing if Makefile.am or > doc/local.mk are newer than Makefile.in, and then calls 'automake > --foreign Makefile' The rules looks complex because they've been generated by automake, this Makefile.in is not written by hand. And I guess automake has put `--foreign` there because foreign is used in Makefile.am: AUTOMAKE_OPTIONS = foreign no-dist But a simple call so `automake -f` (or `autoreconf -f`) just works, as automake picks up the foreign option from AUTOMAKE_OPTIONS, so a human or an external script who wants to regenerate things would probably just use that. > The bot I want to put in place would regenerate things as they are > supposed to be, then build and run the testsuite to make sure that > what is supposed to be committed would work (if the committer > regenerates everything correctly) For your job, would it be fine to just force-regenerate everything and ignore timestamps (just like the buildbot's autoregen job wants to do)? It would waste a few cycles, but it would be much simpler. Simon
Re: [RFC] add regenerate Makefile target
On 2024-03-15 10:25, Tom Tromey wrote: > gdb used to use a mish-mash of different approaches, some quite strange, > but over the last few years we standardized on Python scripts that > generate files. They're written to be seamless -- just invoke in the > source dir; the output is then just part of your patch. No special > configure options are needed. On the whole this has been a big > improvement. The two that come in mind for gdb are (there are more but I don't know them): - gdb/gdbarch.py - gdb/make-target-delegates.py gdbarch.py used to be a shell script, but moving it to Python made it much faster, easier to maintain (as in change how gdbarch hooks are generated) and easier to update (as in modifying the gdbarch hook list). Now, a wishlist I have for generated files (I might do that one day): - Make all generated files (regardless of if they are checked in the repo or not) end with the -gen suffix (like gdbarch-gen.h), so it's easier to know that a file is generated. - make all generator tools have the same name pattern (e.g. make-gdbarch.py, make-target-delegates.py, make-init-c.sh, etc). Simon
Re: Patches submission policy change
On 4/3/24 4:22 AM, Christophe Lyon via Gdb wrote: > Dear release managers and developers, > > TL;DR: For the sake of improving precommit CI coverage and simplifying > workflows, I’d like to request a patch submission policy change, so > that we now include regenerated files. This was discussed during the > last GNU toolchain office hours meeting [1] (2024-03-28). > > Benefits or this change include: > - Increased compatibility with precommit CI > - No need to manually edit patches before submitting, thus the “git > send-email” workflow is simplified > - Patch reviewers can be confident that the committed patch will be > exactly what they approved > - Precommit CI can test exactly what has been submitted > > Any concerns/objections? > > As discussed on the lists and during the meeting, we have been facing > issues with testing a class of patches: those which imply regenerating > some files. Indeed, for binutils and gcc, the current patch submission > policy is to *not* include the regenerated files (IIUC the policy is > different for gdb [2]). > > This means that precommit CI receives an incomplete patch, leading to > wrong and misleading regression reports, and complaints/frustration. > (our notifications now include a warning, making it clear that we do > not regenerate files ;-) ) > > I thought the solution was as easy as adding –enable-maintainer-mode > to the configure arguments but this has proven to be broken (random > failures with highly parallel builds). I tried to workaround that by > adding new “regenerate” rules in the makefiles, that we could build at > -j1 before running the real build with a higher parallelism level, but > this is not ideal, not to mention that in the case of gcc, configuring > target libraries requires having built all-gcc before, which is quite > slow at -j1. > > Another approach used in binutils and gdb builtbots is a dedicated > script (aka autoregen.py) which calls autotools as appropriate. It > could be extended to update other types of files, but this can be a > bit tricky (eg. some opcodes files require to build a generator first, > some doc fragments also use non-trivial build sequences), and it > creates a maintenance issue: the build recipe is duplicated in the > script and in the makefiles. Such a script has proven to be useful > though in postcommit CI, to catch regeneration errors. > > Having said that, it seems that for the sake of improving usefulness > of precommit CI, the simplest way forward is to change the policy to > include regenerated files. This does not seem to be a burden for > developers, since they have to regenerate the files before pushing > their patches anyway, and it also enables reviewers to make sure the > generated files are correct. > > Said differently, if you want to increase the chances of having your > patches tested by precommit CI, make sure to include all the > regenerated files, otherwise you might receive failure notifications. > > Any concerns/objections? We already do that for GDB (include generated files), and it works fine. I sometimes have a patch that exceeds the mailing list limit (400k?), but it seems like the only consequence is that someone needs to approve it (thanks to whoever does that). Simon
Re: Patches submission policy change
On 2024-04-04 17:35, Mark Wielaard wrote: > Hi Christophe, > > On Wed, Apr 03, 2024 at 10:22:24AM +0200, Christophe Lyon via Gdb wrote: >> TL;DR: For the sake of improving precommit CI coverage and simplifying >> workflows, I’d like to request a patch submission policy change, so >> that we now include regenerated files. This was discussed during the >> last GNU toolchain office hours meeting [1] (2024-03-28). >> >> Benefits or this change include: >> - Increased compatibility with precommit CI >> - No need to manually edit patches before submitting, thus the “git >> send-email” workflow is simplified >> - Patch reviewers can be confident that the committed patch will be >> exactly what they approved >> - Precommit CI can test exactly what has been submitted >> >> Any concerns/objections? > > I am all for it. It will make testing patches easier for everyone. > > I do think we still need a better way to make sure all generated files > can be regenerated. If only to check that the files were generated > correctly with the correct versions. The autoregen buildbots are able > to catch some, but not all such mistakes. > > wrt to the mailinglists maybe getting larger patches, I think most > will still be under 400K and I wouldn't raise the limit (because most > such larger emails are really just spam). But we might want to get > more mailinglist moderators. > > gcc-patches, binutils and gdb-patches all have only one moderator > (Jeff, Ian and Thiago). It would probably be good if there were > more. > > Any volunteers? It shouldn't be more than 1 to 3 emails a week > (sadly most of them spam). I can help with the various gdb mailing lists. Simon
Re: Updated Sourceware infrastructure plans
On 2024-04-22 22:55, Jason Merrill via Overseers wrote: > On Mon, Apr 22, 2024 at 11:42 AM Tom Tromey wrote: > >>> "Frank" == Frank Ch Eigler writes: >> [...] I suggest that a basic principle for such a system is that it should be *easy* to obtain and maintain a local copy of the history of all pull requests. That includes all versions of a pull request, if it gets rebased, and all versions of comments, if the system allows editing comments. A system that uses git as the source of truth for all the pull request data and has refs [...] >> >> Frank> Do you know of a system with these characteristics? >> >> Based on: >> >> >> https://gerrit-review.googlesource.com/Documentation/dev-design.html#_notedb >> >> ... it sounds like this is what gerrit does. >> > > Someone mentioned earlier that gerrit was previously tried unsuccessfully. > I think this is a common pattern in GCC at least: someone has an idea for a > workflow improvement, and gets it working, but it isn't widely adopted. I > think this is a problem with the "if you build it he will come" model > rather than with any particular technology; people are more or less > comfortable with their current workflow and uninclined to experiment with > someone else's new thing, even if it could eventually be a big improvement. Agreed. Gerrit has many nice features, but using it would require doing some compromises over some things that some community members consider very important. Would have to give up some things we take for granted today, such as being able to interact 100% by email. But staying with the current way of working because we can't find another way of working that checks 100% of the checkboxes also has an opportunity cost. I doubt we'll ever find a system that checks absolutely all the checkboxes, but that doesn't mean that the current system is the best. > I think that the office hours, for both sourceware and the toolchain, offer > a better path for building enthusiasm about a new direction. As someone who pushed to try Gerrit back then, I'd be happy to chat at the next office hours (if you remind me). > Is gerrit still installed? Hum yes actually, you can check `gnutoolchain-gerrit dot osci dot io`. But it should really be taken down, it's not responsible to leave an unattended outdated web service like that. Simon
Re: Updated Sourceware infrastructure plans
On 2024-04-23 11:08, Tom Tromey wrote: >> Indeed. Though Patchwork is another option for patch tracking, that >> glibc seem to be having success with. > > We tried this in gdb as well. It was completely unworkable -- you have > to manually clear out the patch queue, meaning it's normally full of > patches that already landed. I know glibc has success with it, but I > wouldn't consider it for gdb unless it gained some new abilities. The thing that Gerrit excels at is tracking the different versions of a given patch, being able to easily diff versions, etc. And then mark a patch as merged once it's committed to master. Patchwork doesn't have this capability built-in (AFAIK). You can try to do some automation, but I doubt that any system based solely on getting patches from a mailing list can ever be as good as something like Gerrit for this. Simon
Re: Updated Sourceware infrastructure plans
On 2024-05-01 16:53, Tom Tromey via Overseers wrote: > Mark> See also https://sourceware.org/bugzilla/show_bug.cgi?id=30997 > Mark> We really should automate this. There are several people running > Mark> scripts by hand. The easiest would be to simply run it from a git > Mark> hook. patchwork comes with a simple script that just calculates the > Mark> hash and pings patchwork, which can then mark the patch associated > Mark> with that hash as committed. If people really believe calculating a > Mark> hash is too much work from a git hook then we can also simply run it > Mark> from builder.sourceware.org. We already run a builder for each commit > Mark> anyway. It would just be one extra build step checking the commit > Mark> against patchwork. > > There's just no possibility this approach will work for gdb. It can't > reliably recognize when a series is re-sent, or when patches land that > are slightly different from what was submitted. Both of these are > commonplace events in gdb. > > Tom IMO, asking to always post the committed version as is (effectively preventing doing "pushed with those nits fixed", or solving trivial merge conflicts just before pushing) just to make patchwork happy would be annoying and an additional burden, and noise on the mailing list. The Change-Id trailer works very well for Gerrit: once you have the hook installed you basically never have to think about it again, and Gerrit is able to track patch versions perfectly accurately. A while ago, I asked patchwork developers if they would be open to support something like that to track patches, and they said they wouldn't be against it (provided it's not mandatory) [1]. But somebody would have to implement it. Simon [1] https://github.com/getpatchwork/patchwork/issues/327
Re: Updated Sourceware infrastructure plans
On 5/2/24 2:47 AM, Richard Biener via Overseers wrote:> We'd only know for sure if we try. But then I'm almost 100% sure that > having to click in a GUI is slower than 'nrOK^X' in the text-mode mail UA > I am using for "quick" patch review. It might be comparable to the > review parts I do in the gmail UI or when I have to download attachments > and cut&paste parts into the reply. It might be also more convenient > for "queued" for review patches which just end up in New state in either > inbox. Speaking of my Gerrit experience. I don't think that it will ever be quite as fast and responsive as whatever terminal application you're using (because web app vs highly optimized native app). But the time saved in patch management, tracking down stuff, diffing patch versions, ease of downloading patches locally to try them you, CI integration, more than make up for it in terms of productivity, in my case. The particular case you describe is just one click in Gerrit. The current version of Gerrit has a "Code review +2" button on the top right, which is equivalent to an "OK" without further comments: https://i.imgur.com/UEz5xmM.png So, pretty quick too. If you want to add a general comment on the patch (a comment not bound to a specific line), typing `a` anywhere within a patch review brings you to the place where you can do that, and you can do `shift + enter` to post. In general, I think that Gerrit has a pretty good set of keyboard shortcuts to do most common operations: https://i.imgur.com/RrREsTt.png Not sure that you mean with the gmail UI and cut & paste part. I don't think you'd ever need to do something like that with Gerrit or similar review system. To put a comment on a line, you click on that line and type in the box. > But then would using gitlab or any similar service enforce the use of > pull requests / forks for each change done or can I just continue to > post patches and push them from the command-line for changes I > can approve myself? Like Ian said, with Gerrit, you can configure a repo such that you're still able to git push directly. If a patch review exists with the same Change-Id (noted as a git trailer in each commit) as a commit that gets directly pushed, that patch review gets automatically closed (marked as "Merged"). So you, as a maintainer with the proper rights, could for instance download a patch review I uploaded, fix some nits and git push directly. Gerrit will mark my patch review as Merged and the final version of the patch review will reflect whatever you pushed. Let's say you spot a typo in the code and want to push a trivial patch, you don't need to create a patch review on Gerrit, you just push directly (if the repo is configured to allow it). On the other hand, creating a patch review on Gerrit is not a big overhead, it's basically one "git push" to a magic remote. It prints you the URL, you can click on it, and you're there. Simon
