From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.freebsd.org (mx2.freebsd.org [96.47.72.81]) by sourceware.org (Postfix) with ESMTPS id E2E4E3858413 for ; Mon, 25 Jul 2022 19:13:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E2E4E3858413 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=FreeBSD.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [96.47.72.80]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits)) (Client CN "mx1.freebsd.org", Issuer "R3" (verified OK)) by mx2.freebsd.org (Postfix) with ESMTPS id 4Ls8nq2Mbmz3tHx; Mon, 25 Jul 2022 19:13:03 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Ls8nq1dGyz3gwf; Mon, 25 Jul 2022 19:13:03 +0000 (UTC) (envelope-from jhb@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1658776383; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=8ZQdNtSgm5Y6bifddkLJ5FTpW3D4uO3C5Bo8n0NrNU8=; b=bMJUrxiJytuDb/oyOtHG3sbejhwlyW4wKM4eFtBhVKRZzIvzESYlad93jmI3XCNH3kuKR6 h9d/gtQkIR/4wGEoYPF5qVyYtSehKE88Lf2Z7X2LoOVGWRxyJ6PaipLzRurMsHxKho5GmM 6GCxeWr22N2K+0CKs/zfxPc39r0ugrqULF37QkBM5YxshUvDqB/DqN1BefbQggIouTHpLp oc6t8KO9I/D7Rang72Mr4x00/g1XslVCda6BjdJ7dLcrIbkoK4c5vYZWEd4yr/tHzr90Vp XvvL85OjVBx2qWTAs3ziczjm0m10z1dg+prnHzitlUPtlW1zDwbtDJu8D3QbWg== Received: from [10.0.1.4] (ralph.baldwin.cx [66.234.199.215]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id 4Ls8np5vPYz1Cjb; Mon, 25 Jul 2022 19:13:02 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: Date: Mon, 25 Jul 2022 12:13:01 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Content-Language: en-US To: Luis Machado , gdb-patches@sourceware.org References: <20220719144542.1478037-1-luis.machado@arm.com> <76097707-ed9e-1556-fac2-1992ab3cabae@FreeBSD.org> From: John Baldwin Subject: Re: [PATCH] Update auxv cache when there is no auxv cached data In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1658776383; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=8ZQdNtSgm5Y6bifddkLJ5FTpW3D4uO3C5Bo8n0NrNU8=; b=baF+1fWVebL1oIZwuWo4Mm5q8FgyH4bZP1b3qMlgE+I1X3ZhVh8RCKFqx6KLDboQVT4Xio EDaXUwB9n1kWaSGMe749JCYcV7TPkDRqPGQiFj3wTmTCbyIoAH0RhPamCckPbcU8zxKFZt ckvPBE/RxV4U3VkYumxMj9i8TjjLCHYfuUNMJzvpvsXLpJToJHGWJT2TwApCQ/K/o56PLP j6CSa+HN9SUhj5ERCIklurFQEpsZdQRBpikpor+CmnWVezz/dDcVwz2JKfqBFBj62UHPGD zv3MVoQpD84B13mpzDJWCPHZx3A20QUlCxjvZ0JIaWqS4yY7MQv0Ybbd2RjVBw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1658776383; a=rsa-sha256; cv=none; b=v9USBvc1YOP8OQYeVTJ0UXgyyE1c2sl8LR2ws349ucSrQp1kfbUxq60nqKFtOIvr0KIOsh Sv3dMCvqn9BUUQLJ5e15lxORQABLKbevF0imKip67jnXJRJdT0cZfq6OVCU48Zih4/4jia 7y29rdRNd8KVAY8a4hx0HpXGVQm7lJgEofr3tiLN3eG3Zm9JU5uH2WcVxUGB2ITpXKm1vV DyJ8T4AFboqDH7buqgKlhBRWtka4sG5ALpQW5DS0emHXyyzWkZ1GgVPbbsR6PvcpwZXFVD a6kA9Y6Kwuec3iBV86IQEshV/rQtnF9CXHtWm9+d4eEFtBOXq8yTy2cLWd/jpQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-Spam-Status: No, score=-3.6 required=5.0 tests=BAYES_00, BODY_8BITS, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Jul 2022 19:13:05 -0000 On 7/25/22 11:03 AM, Luis Machado wrote: > On 7/25/22 17:05, John Baldwin wrote: >> On 7/19/22 7:45 AM, Luis Machado via Gdb-patches wrote: >>> While adding support for MTE corefiles and running the MTE corefile tests, >>> I noticed a strange situation where loading the symbol file + core file >>> through the command line has a different behavior compared to firing up >>> GDB, loading the symbol file with the "file" command and then loading the >>> core file with the "core" command. >>> >>> I tracked this down to gdb/auxv.c:get_auxv_inferior_data returning a valid >>> info struct, but with no auxv data. >>> >>> We've been doing the auxv caching for a while now, but sometime between >>> enabling auxv data caching and now, we turned the auxv data into an optional >>> value. >> >> The commit to use gdb::optional<> was this one in 2018: >> >> commit 9018be22e022e6db2ba07c4e407c7244022bc69a >> Author: Simon Marchi >> Date:   Sat Apr 7 13:19:12 2018 -0400 >> >>     Make target_read_alloc & al return vectors >>     This patch started by changing target_read_alloc_1 to return a >>     byte_vector, to avoid manual memory management (in target_read_alloc_1 >>     and in the callers).  To communicate failures to the callers, it >>     actually returns a gdb::optional. >>     Adjusting target_read_stralloc was a bit more tricky, since it wants to >>     return a buffer of char, and not gdb_byte.  Since you can't just cast a >>     gdb::byte_vector into a gdb::def_vector, I made >>     target_read_alloc_1 templated, so both versions (that return vectors of >>     gdb_byte and char) are generated.  Since target_read_stralloc now >>     returns a gdb::char_vector instead of a gdb::unique_xmalloc_ptr, a >>     few callers need to be adjusted. >> >> But looking at it's diff, it was always caching negative results even >> before this change.  This is the relevant hunk of that commit: > > True. I think my analysis went bad when I mixed up data before (a pointer) the optional conversion and after (an optional vector). > > So we used to access the contents of auxv through a data pointer, but now we access it through data->data (). >> >> @@ -358,9 +356,8 @@ get_auxv_inferior_data (struct target_ops *ops) >>    info = (struct auxv_info *) inferior_data (inf, auxv_inferior_data); >>    if (info == NULL) >>      { >> -      info = XCNEW (struct auxv_info); >> -      info->length = target_read_alloc (ops, TARGET_OBJECT_AUXV, >> -                    NULL, &info->data); >> +      info = new auxv_info; >> +      info->data = target_read_alloc (ops, TARGET_OBJECT_AUXV, NULL); >>        set_inferior_data (inf, auxv_inferior_data, info); >>      } >> >> So even before it seems that we would have cached a "negative" result. >> >> I believe your change will mean that we will keep asking over and over >> for the auxv data if a target doesn't support it and that it will only >> support positive caching.  I wonder if a recent change means that in >> your test case something is trying to fetch the AUXV vector before the >> target can read it, e.g. trying to fetch AT_HWCAP* when determining the >> target description for a core file or the like?  It seems like for >> the core target in corelow.c it should be fine to fetch AUXV data in >> the gdbarch_core_read_description hook though.  Did you try setting a >> breakpoint for when this function was called to see when it is being >> cached? > > Yes, we do try to fetch data from auxv before it exists. I think it's always been this way for this particular case. > >> >> I wonder if the issue is that parsing the symbol file without a core >> is trying to fetch hwcap actually.  Presumably a breakpoint would let >> you see that case?  Maybe the fetch of hwcap needs to be guarded by >> something like "target_has_execution" or the like? >> > > No, it is trying to fetch AT_ENTRY to figure out the relocation that should be applied to the executable (if any). > > When we finally load the (native) corefile, then we have access to the AUXV data through the corefile data. > > This doesn't reproduce with a gcore corefile due to the fact that we now include the target description GDB was using when > the corefile was generated. > > Here's the backtrace: > > -- > > #0 get_auxv_inferior_data (ops=0xaaaaad3b0760 ) at ../../../repos/binutils-gdb/gdb/auxv.c:359 > #1 0x0000aaaaab28cdc8 in target_auxv_search (ops=0xaaaaad3b0760 , match=0x9, valp=0xffffffffe988) > at ../../../repos/binutils-gdb/gdb/auxv.c:381 > #2 0x0000aaaaab920110 in svr4_exec_displacement (displacementp=0xffffffffebc0) at ../../../repos/binutils-gdb/gdb/solib-svr4.c:2482 > #3 0x0000aaaaab920e4c in svr4_relocate_main_executable () at ../../../repos/binutils-gdb/gdb/solib-svr4.c:2878 > #4 0x0000aaaaab921008 in svr4_solib_create_inferior_hook (from_tty=1) at ../../../repos/binutils-gdb/gdb/solib-svr4.c:2933 > #5 0x0000aaaaab9288a0 in solib_create_inferior_hook (from_tty=1) at ../../../repos/binutils-gdb/gdb/solib.c:1285 > #6 0x0000aaaaab9747cc in symbol_file_command (args=0xfffffffff6c6 "~/aarch64-mte-gcore", from_tty=1) > at ../../../repos/binutils-gdb/gdb/symfile.c:1655 > #7 0x0000aaaaab500fc4 in file_command (arg=0xfffffffff6c6 "~/aarch64-mte-gcore", from_tty=1) at ../../../repos/binutils-gdb/gdb/exec.c:555 > #8 0x0000aaaaab3519f0 in do_simple_func (args=0xfffffffff6c6 "~/aarch64-mte-gcore", from_tty=1, c=0xaaaaad786a60) > at ../../../repos/binutils-gdb/gdb/cli/cli-decode.c:95 > #9 0x0000aaaaab3572c8 in cmd_func (cmd=0xaaaaad786a60, args=0xfffffffff6c6 "~/aarch64-mte-gcore", from_tty=1) > at ../../../repos/binutils-gdb/gdb/cli/cli-decode.c:2516 > #10 0x0000aaaaab9f0ed4 in execute_command (p=0xfffffffff6d8 "e", from_tty=1) at ../../../repos/binutils-gdb/gdb/top.c:699 > #11 0x0000aaaaab68c75c in catch_command_errors (command=0xaaaaab9f0984 , > arg=0xfffffffff6c1 "file ~/aarch64-mte-gcore", from_tty=1, do_bp_actions=true) at ../../../repos/binutils-gdb/gdb/main.c:513 > #12 0x0000aaaaab68c9ac in execute_cmdargs (cmdarg_vec=0xfffffffff080, file_type=CMDARG_FILE, cmd_type=CMDARG_COMMAND, ret=0xffffffffefd4) > at ../../../repos/binutils-gdb/gdb/main.c:608 > #13 0x0000aaaaab68df18 in captured_main_1 (context=0xfffffffff280) at ../../../repos/binutils-gdb/gdb/main.c:1298 > #14 0x0000aaaaab68e0c0 in captured_main (data=0xfffffffff280) at ../../../repos/binutils-gdb/gdb/main.c:1319 > #15 0x0000aaaaab68e11c in gdb_main (During symbol reading: const value length mismatch for 'sigall_set', got 128, expected 0 > args=0xfffffffff280) at ../../../repos/binutils-gdb/gdb/main.c:1344 > #16 0x0000aaaaab1655ec in main (argc=5, argv=0xfffffffff418) at ../../../repos/binutils-gdb/gdb/gdb.c:32 > > -- > > I think this was a latent bug and I just happened to run into it. It doesn't make sense to cache the output of the auxv from a target that > doesn't have auxv data. But we don't want to keep requesting things over and over again. Hmmm. I think in some ways what we would like is something like: if (!target_has_all_memory ()) return; before calling svr4_relocate_main_executable. However, we don't yet have a target_has_all_memory() wrapper function. Remote, process_stratum (native), and core file targets all have has_all_memory() overrides, but the exec target does not. We can't use target_has_memory() because the exec target returns true for that. It doesn't really make sense to relocate the main executable when we don't have an address space. Maybe that is the better question to ask if it's askable? That is, does the current inferior have a real address space, or is it just a plain symbol file via the exec target without a real address space yet. Looking around though, there doesn't seem to be a good way to ask that question. Hmmm, I've dug a bit more though at this from a different angle in that attaching the core file should be flushing the auxv cache and I'm curious why it isn't. The 'inferior_appeared' observer event (which clears the auxv cache) should be firing when a core file is attached via 'inferior_attached' from add_to_thread_list in corelow.c. I wonder if the combined case on the command line doesn't trigger inferior_appeared for some reason when opening the core file? If so, fixing that might fix a few other things as well. In terms of the original patch, I find it a bit odd to replace the existing info structure vs just replacing the data member and/or if we want to disable negative caching we should perhaps avoid emplacing the object if the fetch of the auxv data from the target fails. -- John Baldwin