Re: [PATCH v2 11/18] Do more DWARF reading in the background

[Tom de Vries <tdevries@suse.de>]

On 11/13/23 18:56, John Baldwin wrote:
> On 11/12/23 11:15 PM, Tom de Vries wrote:
>> On 11/12/23 21:25, Tom Tromey wrote:
>>> + /* How much effort should be put into each worker. */ + const size_t
>>> size_per_thread = total_size / n_worker_threads;
>>
>> I've tested the patch series on x86_64-linux, and ran into trouble in
>> one test-case:
>> ...
>> (gdb) file
>> /data/vries/gdb/leap-15-4/build/gdb/testsuite/outputs/gdb.dwarf2/dw2-dummy-cu/dw2-dummy-cu^M
>> Reading symbols from
>> /data/vries/gdb/leap-15-4/build/gdb/testsuite/outputs/gdb.dwarf2/dw2-dummy-cu/dw2-dummy-cu...^M
>> /data/vries/gdb/src/gdb/dwarf2/read.c:5000: internal-error: do_reading:
>> Assertion `iter != last' failed.^M
>> A problem internal to GDB has been detected,^M
>> further debugging may prove unreliable.^M
>> ----- Backtrace -----^M
>> ERROR: Couldn't load dw2-dummy-cu into GDB (GDB internal error).
>> Resyncing due to internal error.
>> (gdb) maint expand-symtab^M
>> 0x5ac476 gdb_internal_backtrace_1^M
>>           /data/vries/gdb/src/gdb/bt-utils.c:122^M
>> 0x5ac519 _Z22gdb_internal_backtracev^M
>>           /data/vries/gdb/src/gdb/bt-utils.c:168^M
>> 0xd11a89 internal_vproblem^M
>>           /data/vries/gdb/src/gdb/utils.c:396^M
>> 0xd11e58 _Z15internal_verrorPKciS0_P13__va_list_tag^M
>>           /data/vries/gdb/src/gdb/utils.c:476^M
>> 0x14ca6f0 _Z18internal_error_locPKciS0_z^M
>>           /data/vries/gdb/src/gdbsupport/errors.cc:58^M
>> 0x72b01b _ZN19cooked_index_worker10do_readingEv^M
>>           /data/vries/gdb/src/gdb/dwarf2/read.c:5000^M
>> ...
>>
>> This fixes the test-case for me:
>> ...
>> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
>> index 385160de0fd..191c2ad7f26 100644
>> --- a/gdb/dwarf2/read.c
>> +++ b/gdb/dwarf2/read.c
>> @@ -4974,7 +4974,8 @@ cooked_index_worker::do_reading ()
>>        = std::max (gdb::thread_pool::g_thread_pool->thread_count (),
>> (size_t) 1);
>>
>>      /* How much effort should be put into each worker.  */
>> -  const size_t size_per_thread = total_size / n_worker_threads;
>> +  const size_t size_per_thread
>> +    = std::max (total_size / n_worker_threads, (size_t)1);
> 
> Presumably total_size was zero in that case? 

No, it was 11.

> Should there be a shortcut
> to bother queueing a task in that case instead perhaps?  Or alternatively
> is the issue that total_size is < n_worker_threads?

Yes, n_worker_threads was 12.

> If the latter, then
> a min size of 1 does seem sensible.
> 

Thanks for the review.

- Tom