From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x102b.google.com (mail-pj1-x102b.google.com [IPv6:2607:f8b0:4864:20::102b]) by sourceware.org (Postfix) with ESMTPS id D99423858D38 for ; Wed, 27 Dec 2023 00:30:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D99423858D38 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org D99423858D38 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::102b ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1703637020; cv=none; b=QGaqwIQEXSDDbhVCpxTYLhnbXuEJ2Hqvz4KQP1ZmBPtO3o8XYD++PEziATXFS6rhv+gjOXqQR1tJcKG+4GEQjwYBE9TioOOM2h6y9v4DyjHMtVRr6iaIbBUI/nKED8m9jwM4F73xyFPv7Uk4t2pHLGUhkrA9Skn92QdwBD03HYU= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1703637020; c=relaxed/simple; bh=Hd5aO/trj2t+k5YynI2XZHnteSEiG43SeVftS2ImduY=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=WodknZOp1p3z4cusYJx92v904G7oQmMaIywWMIykVMowGtfSlxsb5EkqnBjp1Nkzhijaf/pbrGbajclYcq9WqUUHqEVwAw/qGTWagsIgw+Q3N6/kb5OdWjzK2RD6c5PzQSj418YntIouOV9wYFgeTFkk0TznLLKMSGCZ2L45SdY= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pj1-x102b.google.com with SMTP id 98e67ed59e1d1-28c0df4b42eso2067958a91.1 for ; Tue, 26 Dec 2023 16:30:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1703637017; x=1704241817; darn=sourceware.org; h=mime-version:message-id:date:in-reply-to:subject:cc:to:from :user-agent:references:from:to:cc:subject:date:message-id:reply-to; bh=JD30676RlpW29Ds5MkepqMWrfWbLchqvKa396oAA6k8=; b=Y9ktbt9tvaCXw9c601NN2O+9SDjxIpTXIRLIzCWNFNY86Ima5gzrPsuQudZECIcv7Z tEUgkbBla6sDJH/KD9/3dbGLEsUJ07kNy16UEHxA091W9WhUHFIHFLX0qVU79djfyLhb fJ+c14+GyLW/qo1YX2sqHHP/7Quh7gWom0CQsJJ9eGssZE7y/579l4DKVU0BZjy/QjYL iEOuIYgEVDoBAOeFHpluEpxMOA67S6bNee5xxW5IZJZfoys+NQYGB9U0VUvzMwuh98fm veP51Y/6m8VH5fHx2nO5LRVzKjXVP8Dux08DapXGv1+WTrLs8JLFM0RD8OWgLhmqMGJF GINg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1703637017; x=1704241817; h=mime-version:message-id:date:in-reply-to:subject:cc:to:from :user-agent:references:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=JD30676RlpW29Ds5MkepqMWrfWbLchqvKa396oAA6k8=; b=Sovt8TMLYi1fBs2Q4pwtZwX5jga58pdcU7JLyB5+sjY7IjvKClP14ayNX44zf2iyQx HuEwbuoDU+v8G5JMwpeoGUKFCOoEGJfnS7Qeh/JiwU+YUii9rrqGhCNCvpuoSlo288t4 YX4IsayQVJoN7ZCdjjVG7PbzNlcH674V+OQoeui7Mk98vXJR5HjIVMEhXrxwbClAI9Mq t+JAljI+Nbl3+TMamyepKte6KblkOW+njI/RaZHr/pJySSAkNs3Om8pGr2rP7OmiJFff aHOCnsepjxtIUMliL+ZALLc4wKGRLU07s6AIU2KlxvgVoy78Vib5bm4R/PNarF0LiRKL puHA== X-Gm-Message-State: AOJu0YyTk4hCSP+MTewTk2VN+x3XOPgjoGv6Gvm3Kg9EHrW8k3X4EuXU PC/ev0JLEVJ6/iH0JbKHnHh4r6i8hoN0MAigzEmDevo54mE= X-Google-Smtp-Source: AGHT+IHJBJ0fr97PVYjH4xmAACRY4/MVnkryXeGLf1lyxcbLGZ7Ape13BPBSGFMqrkH6Zypvp6L8sg== X-Received: by 2002:a17:90a:cb08:b0:28c:1739:ac with SMTP id z8-20020a17090acb0800b0028c173900acmr7680690pjt.12.1703637016628; Tue, 26 Dec 2023 16:30:16 -0800 (PST) Received: from localhost ([2804:14d:7e39:8470:b425:ec2d:a09a:544f]) by smtp.gmail.com with ESMTPSA id f2-20020a170902860200b001d33c85ce1bsm10622850plo.2.2023.12.26.16.30.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 26 Dec 2023 16:30:16 -0800 (PST) References: <20231028002008.1105723-1-amerey@redhat.com> <20231028002008.1105723-5-amerey@redhat.com> User-agent: mu4e 1.10.8; emacs 29.1 From: Thiago Jung Bauermann To: Aaron Merey Cc: aburgess@redhat.com, gdb-patches@sourceware.org Subject: Re: [PATCH 4/4 v5] gdb/debuginfod: Add .debug_line downloading In-reply-to: <20231028002008.1105723-5-amerey@redhat.com> Date: Tue, 26 Dec 2023 21:30:14 -0300 Message-ID: <875y0kcwrt.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-4.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Aaron Merey writes: > -/* Read directory or file name entry format, starting with byte of > - format count entries, ULEB128 pairs of entry formats, ULEB128 of > - entries count and the entries themselves in the described entry > - format. */ > + > +/* Like read_formatted_entries but the .debug_line and .debug_line_str This function is also called read_formatted_entries, so the comment is a bit confusing to me. Perhaps start with "Like the other read_formatted_entries, but ..."? > + are stored in LINE_BUFP and LINE_STR_DATA. This is used for cases > + where these sections are read from separate files without necessarily > + having access to the entire debuginfo file they originate from. */ > > static void > -read_formatted_entries (dwarf2_per_objfile *per_objfile, bfd *abfd, > - const gdb_byte **bufp, struct line_header *lh, > - unsigned int offset_size, > - void (*callback) (struct line_header *lh, > - const char *name, > - dir_index d_index, > - unsigned int mod_time, > - unsigned int length)) > +read_formatted_entries > + (bfd *parent_bfd, const gdb_byte **line_bufp, > + const gdb::array_view line_str_data, > + struct line_header *lh, > + unsigned int offset_size, > + void (*callback) (struct line_header *lh, > + const char *name, > + dir_index d_index, > + unsigned int mod_time, > + unsigned int length)) > { > @@ -166,36 +173,48 @@ read_formatted_entries (dwarf2_per_objfile *per_objfile, bfd *abfd, > switch (form) > { > case DW_FORM_string: > - string.emplace (read_direct_string (abfd, buf, &bytes_read)); > + string.emplace (read_direct_string (parent_bfd, buf, &bytes_read)); > buf += bytes_read; > break; > > case DW_FORM_line_strp: > { > - const char *str > - = per_objfile->read_line_string (buf, offset_size); > + if (line_str_data.empty ()) > + error (_("Dwarf Error: DW_FORM_line_strp used without " \ > + "required section")); Is error the correct response here? Or should the bad form just be complained about and ignored? > + if (line_str_data.size () <= offset_size) I don't understand this check. Shouldn't it be the following? if (line_str_data.size () <= str_offset) > + error (_("Dwarf Error: DW_FORM_line_strp pointing outside " \ > + "of section .debug_line")); IIUC, the message should say ".debug_line_str" instead of ".debug_line". Also, same question about error vs complaint. > + > + ULONGEST str_offset = read_offset (parent_bfd, buf, offset_size); > + > + const char *str; > + if (str_buf[str_offset] == '\0') > + str = nullptr; > + else > + str = (const char *) (str_buf + str_offset); > string.emplace (str); > buf += offset_size; > + break; > } > - break; > +gdb::array_view > +mapped_debug_line::read_debug_line_separate > + (char *filename, std::unique_ptr *resource) filename can be const char *. > +{ > + if (filename == nullptr) > + return {}; > + > + try > + { > + line_resource_mmap *mmap_resource > + = new line_resource_mmap (filename); > + > + resource->reset (mmap_resource); > + > + return gdb::array_view > + ((const gdb_byte *) mmap_resource->mapping.get (), > + mmap_resource->mapping.size ()); > + } > + catch (const gdb_exception &except) > + { > + exception_print (gdb_stderr, except); > + } > + > + return {}; > +} > + > +/* See read.h. */ > + > +bool > +mapped_debug_line::read_debug_line_from_debuginfod (objfile *objfile) > +{ > + const bfd_build_id *build_id = build_id_bfd_get (objfile->obfd.get ()); > + if (build_id == nullptr) > + return false; > + > + gdb::unique_xmalloc_ptr line_path; > + scoped_fd line_fd = debuginfod_section_query (build_id->data, > + build_id->size, > + bfd_get_filename > + (objfile->obfd.get ()), > + ".debug_line", > + &line_path); > + > + if (line_fd.get () < 0) > + return false; > + > + gdb::unique_xmalloc_ptr line_str_path; > + scoped_fd line_str_fd = debuginfod_section_query (build_id->data, > + build_id->size, > + bfd_get_filename > + (objfile->obfd.get ()), > + ".debug_line_str", > + &line_str_path); > + > + line_data = read_debug_line_separate (line_path.get (), &line_resource); > + line_str_data = read_debug_line_separate (line_str_path.get (), > + &line_str_resource); > + > + const gdb_byte *line_ptr = line_data.data (); > + > + while (line_ptr < line_data.data () + line_data.size ()) > + { > + line_header_up lh > + = dwarf_decode_line_header (objfile->obfd.get (), > + line_data, line_str_data, > + &line_ptr, false, > + nullptr, nullptr); > + line_headers.emplace_back (lh.release ()); This line is destroying a line_header_up just to construct a new one to put in the vector. I believe you can use std::move here instead to avoid the extra work. > + } > + > + return true; > +} > +#endif /* !HAVE_SYS_MMAN_H */ -- Thiago