From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (gnu.wildebeest.org [45.83.234.184]) by sourceware.org (Postfix) with ESMTPS id 5CBD7384AB61 for ; Wed, 10 Apr 2024 19:47:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5CBD7384AB61 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=klomp.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 5CBD7384AB61 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=45.83.234.184 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712778490; cv=none; b=LmogRn2uGy0ndJ7jLHuY9Tc8ageBoqOZFOsCwdSjGdH6RCZJfR0AOzwDGL1+zFtOkeTwmW60RLXAQA+jLAfbEk4x0esDPBhMkWw7LIMCyNj4fLyQH9Q9HkjGpX0tji5cMiwpVoM76jcZbOEHnKBkOEDtssoDMxa/tH/Wo/P4GeI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712778490; c=relaxed/simple; bh=PI7pmYRtWlMC24csvVZ2hWnoylDHzXzW50jso2A2cpc=; h=Message-ID:Subject:From:To:Date:MIME-Version; b=L/AKG5Vnp6esinxgjv02tHuMAg6FdMZcJAIOYSy8q3nxk/YDYLXHyodtGz0YmN/VrIIQkXc+ZRaAjQNP/HVbo4SDRyv6kTghMnItp0fa85iBXEhSoaD/3gznZYrhU5i9qk6zeqsubDDQ9mRp9qiw647O1Sgm9haRQJfsOVx0VC8= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from r6.localdomain (82-217-174-174.cable.dynamic.v4.ziggo.nl [82.217.174.174]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id 334893000595; Wed, 10 Apr 2024 21:47:57 +0200 (CEST) Received: by r6.localdomain (Postfix, from userid 1000) id 9BDA53404BD; Wed, 10 Apr 2024 21:47:56 +0200 (CEST) Message-ID: <2a6f04357e6cb900fe055939673a7abc542ae333.camel@klomp.org> Subject: Re: [PATCH] libdw: dwarf_getsrcfiles should not imply dwarf_getsrclines From: Mark Wielaard To: Aaron Merey , elfutils-devel@sourceware.org Date: Wed, 10 Apr 2024 21:47:56 +0200 In-Reply-To: <20240410034539.164402-1-amerey@redhat.com> References: <20240410034539.164402-1-amerey@redhat.com> Autocrypt: addr=mark@klomp.org; prefer-encrypt=mutual; keydata=mQINBFxDPtIBEAC8xePaWvq5cDMBYtrpdmR20YX5xrDXUeHgueSVE9Mw8yCan2Cq1Ac1jHYnXxp4Jj3q4tIS9Jq2oAbqxyvBMdJYqEz4z709eDnYBacZQcGqojLh3HI2P7lmmKxkL8rS3Q3Ug05mYT+MwgmRvIO1+kwQTcq1AeB9z9/zikgY1Jv1R86gH8G84OPbJUowdE/mT3pQ+2UQnarCVPJ3WJtZNDXPRzskk0t5h+Mg5RtX+COoHfsvWHiEUTmHMfynw49GG/YF6jLSVzMlKMz3jdOePIArpm2BNUu8DvEn9at6daKR4Ah+ujDd08l9j8wFhJnenn/9+ENjm9kOGQWOmH/fEIOlMAATVdZEfHVfAWbgICPSrPyi+v3ACE4uEoaw85LgbAAbhzdswlLezLxS7LLTpfDZUFZfkho1MSGXaCQ475/iVAeuxi61B2VcmH4lOSH7HYNkMY8ggGk2/WG35eq8PZme8PvXUmLu+f2jzy9XFekIr+/Ks2TchCTYCw6bhSLPa19HapCxvWXgNcYzZ8jULqBXsAfj4NnzBTn6u5nsJ1reA8GoO9vLaQf3LDgr+UY/z+6N474lAVfr0eIdWzkWPvX8fcBCfiB944rPr+Q50RUfc9ngIKP4JsflhXTn601aL4r6qkCcBVqO/eRhb4vCAUo7csemTKzI/05ooGfQtZ3O5QARAQABtB5NYXJrIFdpZWxhYXJkIDxtYXJrQGtsb21wLm9yZz6JAlEEEwEKADsCGwEFCwkIBwIGFQoJCAsCBBYCAwECHgECF4AWIQTsPP6I9soHiHdPXB0apEvmSd52CgUCXE37mQIZAQAKCRAapEvmSd52CuO9D/9Fi6LOrU+iYHjfIk+wT8jyhF1YNATnooa5W7y/4QlXOIyKmkXM/0faH1hZNGf4qVK4dBUewuhALMEzudkXEhzudg9KpB9SaHZNR5DZ+YHo204zJ84P+aJa7F8FOScbNAiG4 pFGC7sQxvtAz0skM0yLsdhNg2tM8lM3n9e/rO4EK7aR55ojzE9pCWhRSx/AKYT7545KzXuCRTky8fRcI8YeNNLPIseoV3QPkf7qNi6YXl0yUHV5gQMCWqgbfdHAljd2+N1RZvdzfEOLVPLX4/dgxb36i9OKkuCAHLoL2UXfzcAElltHQundNi/xYzSizzEYBeIbVrbuqiJP1zmiPUKxHibkU3ThZZUbonKRNVPQe1hO47Cxyj1RxXl6Nt9uda3W9ow6Kr96Bjs3WVBSqsuohqaAlAxC6RccslrEw/7N7l8S423LJI6ZV+FvyJzmSAqkLNz/tuFSMj76uH4s1dLbRv8K4fcw1vZgqy/4jIhBFycn29hMNvImKbMnLDwC7K92lBGQ6hp75/0Hf1qHOpDaiyV9Qqzr3sTOMXJiYm3ac5bDqJb9Mi5YPNB2OD3w3bDMGT5+eWjmw9RiYT5gNjY6nZhDiQS/PtIc6l3i2GaBjSdurwj47TlCWALj3ZiiEKiybanK5/YXdVXGchLnoNkxeI1YcANZhX60FWEyaHZsa7QbTWFyayBXaWVsYWFyZCA8bWp3QGdudS5vcmc+iQJOBBMBCgA4FiEE7Dz+iPbKB4h3T1wdGqRL5knedgoFAlxN+lMCGwEFCwkIBwIGFQoJCAsCBBYCAwECHgECF4AACgkQGqRL5knedgo1bhAArI7kReYq4YtaxS8Pxb5MdPxiQVtvfkbycWCZ4owzPeEIkJqcbadNUiGSqCRR2xeT4kuzFZWILiZfBTwHwFM/bXRDK/FOn7F8aqUAV1tq2W70Z7BUpTwpAv7Xm5YvsfbTBZmllJltEiIrKIzULCtRKKVXgtOKg0sd/W2aXwyl+OX+PVzu4mXXNEkO10J7VpnCvjyaJNeKgeJYQLizSWdEf7i6RX31yC29+GsSqikaOHdfxJMM+bo/x/aCuYlgDB+OQ6LZzpXZO0C8B5SMgMfZaK1rxDtUtViajSyOFJ4Ig6 bcgc5qDCLnk407oEN1yBWps867uN/Bi4Dk+xh691feGsyq95DvPis2Ut+0X0/Wi/uLg3uu/X5EcNHynwht7KaGCLeuOZKxvzfeudNeyKFX34HtFyE/2k9LR0mFX8XnXQGBD9psOxcd2K8Rku9BjjKDZ/vf53sMh5vxUNo+zkd+5dLZWPnLrhkfQrepDBP+Tc/6W0VSZCP5/nKX6GjPwmELtZj4jGf33tgfNMJrmxGUjpDxtiJc7OroNC4he3F5AF4RNRa5VvHs6ah57swVvKyJmLH5mxxKIn39PspOhzVxSbkWNPLS+km2InPum+fmYKQL6IrHcqt/ecrR7o9GRgI0cJjLJ+wv93ti+gxsUWPbAUBaJPk24omIpQafFT/YAEW0Hk1hcmsgV2llbGFhcmQgPG1qd0ByZWRoYXQuY29tPokCTgQTAQoAOBYhBOw8/oj2ygeId09cHRqkS+ZJ3nYKBQJcTfqnAhsBBQsJCAcCBhUKCQgLAgQWAgMBAh4BAheAAAoJEBqkS+ZJ3nYK6JIP/jGq2adAkk1IPsVx7X5argCXaI5o3aTG+N6BPMvizGY4CyqTBmYAg3qRPWN4U+wxNFgzQ3nPfptJK6EDBAY4Lw+kKR4o8o5zPyjAjbc9/be5bvTwMFmMbzWxGWoJ4ch9W+wD4W+W8ssnJDbVi5Btp7kKyUgg3KP7oUQpxZ9MTgmKEmPuce9zOQ78q+LIPbkhI9zCS/1VCHkeVyn+TrnACoHx7sKgJoOwjVlcbW3S0sdCuaGg3+VLI3v3IpQ56UXIX6RVeLX9zVDZnAq72Age4HHcxjX97j16iL5ZhZRc24J5tpSkJgHt+RASOKHJGPIivCqKbQKhYc1G4MbFbGzclaLTXya3Q4ekFzo4ohd2ga7mgjG0SJ+aIU5wCYxEUDsqJLm+uH/nbQzXTxIoQhH5yub4OhW88z6LpwPGhLvzS2SuPJIXAlbGyrH70/uRZWkwKF/8mQjMCsLEYkZ 9DRB815rUTc9CJkkeOlPXQUbxr2fDAgi0j3yAUxlrC7jESO/zUJrICbxChYAx9KMWG/2PsKbrGAAMKiC7+q6mY09Q63F/g1DEF2sb+bguMdWc7SEj64jFUf9wJ+vjU1F321Bkh/QWMABv6n+7EFkwnNkylCR5H1boOHO03TNT0jyLbBECR7/Mtpwt46c4+n9EPCmQyvdU3MVPORvZge1hzvuvfo22uQENBFxDuhkBCAC19Q021v7kTuwYKwEmbqQC5wvmbEMT4ldvQ8gWCUIFL9kTxM67IF0annsys+rrAyqqFUTq2onVmgjciu9upl6uDdV3wivCBEDN9ZLZAVHTSviiXDhnHUSg6EhCdZKhal9DKAi+vGSLSe14e2Kfoe4c6R0yDVI+Dn0OfUhlMXu2NoDSFLAdHsDHSCrE6xKO+BNgL2MPuMeXLhNitNIVrykoZMkFrUMcMsHrvrk05ah87RQO1e2ljenn8qxPRLdOVWc0TJiosjiy04vwDAYNUCPDL5W2Mp2bv2AeTPCzF1qkDnGKZEqV2peWKCPB608lS1icw5oKtOl50PSgzTdaLVRXABEBAAGJAjYEGAEKACAWIQTsPP6I9soHiHdPXB0apEvmSd52CgUCXEO6GQIbDAAKCRAapEvmSd52Cpy8D/9tq4BQ3VwrDNCxycALqWvZSPv/AgsT6hRvQsLc6Yp0FEtz+frFPLWt7bylMrzKItpsr0G2FofWw0yNyHNYPmGlCi+SrWLJnUTEm5TZgwT+9kLt/mJ4B0J1gHkknXSo91S84DPaik9CH0GmXIQyPANkDDlmp9W/Hk8oKxxvCx+SSsZ6ANXakcNVg/w4MhDW2HowW4sBvtltOFSgPRs9zISiNw//GYjeYrdOOnieMhszwpjQuK5XYnDhwiSap2D8nQlD/VpAa2CvE/fOFV2CJyKZfE0J8v5DZOU+SUwnty1f52ZA1s/OCysaK1LLdCXz3bQiybQZhobcAneBVZFl Nzf6xpR+pGtw3OVSyLQo4LSQf4lFszNy8FfE+BJ1/yUWFBjljLwIHd4IW7Y17PugAc19fQ23krOIc3O4qsuYzqdhzYzqGbPvf7fY3Tz0BNcW5885KEQJH7VJJLqpf3EELhmkLBONYiF10iggFSmn8WSQWbXm0kGRETvAzf+FYcJsKDu9QASDRNck8J20ZJGVLbZNdP+VuLOXCDAkSGIxi91TLi6bY0Mb2yNRgAq6cnIJUTAbcnw05BLxRW+e8AS3HodjZHWzAMDPpZn5TFfJOXdDhdeePVGgkypxwnbeyTT3OjUEh37vr+XIgrTMpz+ZNpHxLr4bJatQEVK3H6Q3ZbQkMbkBDQRcQ7q3AQgAqSM4Wx4QvvCIf8is+57mLJhceB2kLt3VR67UFZC0ywcr5V0pvTuu2U1oUB+BVYC/A9UdnvWTyDef3xTCx0hAiiFhlMe6CkODOalmxI+KwPxD276+70tcxd8vR2FJviDQKw96f2hlLAnxR47GUp3cPfIgVfkvIXnXLMUJQvBhXeXqgYhOcAplI677n/zTeFjBtd/JqtyDoJ0De1odEyC+ZZD/Jo5q80Sydhvb99BHQMgLTJTJPW1iRV2AK4xfBjxOMwqml9Lx4HRIpV/IHs3MTyhEpEA+I/eKpO6UxApHWHZ76Zm8BL8RwnfFaXjMueRhIGMFtJnLuNFc5mOLXa3uhwARAQABiQNsBBgBCgAgFiEE7Dz+iPbKB4h3T1wdGqRL5knedgoFAlxDurcCGwIBQAkQGqRL5knedgrAdCAEGQEKAB0WIQQSdoqWeVmQEHoNL9/8V+PMrNmaeAUCXEO6twAKCRD8V+PMrNmaeEvuB/92qMj2mQN3CXRQUTlmzVNUJLJAwzjRDoSt3kqDrACJ2N8JLSxWFeHmEmrrmFPUmXfBUkT+F2W+OrsJlUtwepuTYROgLNZebFQdjB38oqsj8RMKb5ikWntRUka2xhSDRBa0IlpxHBWLHS8nEx1x4 HB4uYRK3IpWShAVmWk7jiATGJLFYJGVo4TBfM27zCty8/GQN/3A2DAJ2OJbiJ12ByTgzztGdhJ69H/QUltkK7eJUGMjPwhpmp07lrolyUurbzaLMQow4SLo/ZIxa0nPC+AoMSk06teichCZwIyiU/70S0c/uL3RFhnTbgWcdQkAVpWdkwFqIES4xG5QLUu85/WT7lMQALJKKuOOpbOeKvyLV16Oo70OTms/LbmXU9+bjCjz7QISuzhI4rua0onjQzBaRXFYkfCjBudWaEpy/wP5wk6QlqxLkeZNCk0TswksLxQjyO2XgBcOnrSsQIEJ7VICG9PDvtVzbrSBYMjoDo58AyniEMVANyUnFYl1YBlFt506PDh86ZEqlpbbReAsYfEuBQdBfJhWph9WZgJDVtEHUAxaiqisvNEbz4xRIAsxX/OxnQMdD09Xs50yvl38ERIadacejtQnAIYeEaUBsgQk3rt0+g9lm6trD7P4FXYhUD9vml6/n8TGB3UJi3lKpX41GSUC1y+oPna8p+EEmrm3BbB4fgnIkfYiEDNogvm2pe7nzUP7sNnE8RcyYcjUoEQ0Uo+HB6fk6NeBGKqaIKVexCcExnWKHvl0DZzGydvKx41nyzFI1sueg34LcWwpGHXzJyhmpjhNe1GOKtVGHCGKhKhppK4ntUZISciGh38wvKuFDohHO3JVZ9AhyRWKTuynzLarBpmvu11TDbv0lfnZcghlWWHNlx8x8DdaEuFWXZTDuVXqGclmeV2hS0LomX33LCB4n0XkZtC9LsmTIsr+ZdVCAXUeX/pJONNxNF8G47lZLLgLWF9beuHWp3u1Io31fzh44TZxm1Z31wCZjOrsL9bvy3xHyDFaDL+/7i6TXsSxtqTXuQENBFxDu6IBCACgVJJnY8zh8uHn8d/E7p4j+9ueTvTHMRYOS0kkGhHBC7JmxCw6/EvbnbTsI0CQeyIJHlmPIqDVgRVjijcTWacd3vIdazzH9sqs65 nl49yMnA23tIya4VWlbHC3J4x/LL84A4GaJO/FVF2vv6hVg3IGbopp5KX+pr6s56TiWddSDqMgjb7rSzjWuNyRK75ToctL7Y/Zn6st3ZioO7LXq3ghkWf8JR7ZaUFIY6P1qS5heiCHP0PxQJSrtpYzH3rKJoHpIkjxnsB/sD0C05cAdlzXBTUVTNLY+DPlQ7FeRkG+VK91briG4tvQ8ohhEiC9HuJu1AKMNWBZ9qeUwsXaJvNzABEBAAGJAjYEGAEKACAWIQTsPP6I9soHiHdPXB0apEvmSd52CgUCXEO7ogIbIAAKCRAapEvmSd52Ch8ZD/9wKuIlaRMSB1AMCwhGPaqXZahrJ649Y0jI4JqpFKv2/U5hKHOG7bihRAeEj7pZzhlgBrkZg1SBdZ3vHs1ufElnfe5RQApdDm93daU5SP29iEivJQxKjF91EfEffl0trxxztBipI5/2D+kaS8cnNVfzo5ZEWy/cd6AShvRVHM7Y2QHc+mlaZhYhBvTtwC6avXNnG55WYgobGENeAwkyD072JF3XrxFb+XkcKxla9yRdWdHxJd5PYJqsKM+nVeJM226OwOyU235gfIhIP6pfGqF9UVH0uFoCYkVkUSjVd96Q+Cj0kdhTOrtLW1OY11d9TBxje42GOtc7X9Zzx1nhwU8rCCErF9/uJIJKlq7I08rMX3rFDTtizwN7g7ZBkDDiZO+BIKQPt/awA9NM+tda02hyfQokBBi+v8b/iKifKIfUaqPDo1PA5uxljdluyX8AXIotKjJXF6Elsiz7bVpcIc0ZXOOFr9ylmtZm51YNmOzDNznEBmol2oBZfsk2G55/QgShHmKUnvzKANBGfnfS/a/K7Hv4sfZAb58Prl6OmQSrkmhzFry/4BNLKq+nd4s8VXkJPpx3Ogf3DoIynqpNF0bwf52U5IgJSNcJN/HrAwhaG1W+Y3LDe7S19M0cUzftEUeq3Jd89hoijC72tdba+BRfW0ncfvEcsk9 QifSU1tvZxQ== Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.50.4 (3.50.4-1.fc39) MIME-Version: 1.0 X-Spam-Status: No, score=-9.5 required=5.0 tests=BAYES_00,GIT_PATCH_0,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,KAM_SHORT,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 List-Id: Hi Aaron, On Tue, 2024-04-09 at 23:45 -0400, Aaron Merey wrote: > dwarf_getsrcfiles causes line data to be read in addition to file data. > This is wasteful for programs which only need file or directory names. > Debuginfod server is one such example. >=20 > Fix this by moving the srcfile reading in read_srclines into a separate > function read_srcfiles. This change improves debuginfod server's max > resident set size by up to 75% during rpm indexing. >=20 > * libdw/dwarf_getsrcfiles.c (dwarf_getsrcfiles): Replace > dwarf_getsrclines and __libdw_getsrclines with > __libdw_getsrcfiles. > * libdw/dwarf_getsrclines.c (read_line_header): New function. > (read_srcfiles): New function. > (read_srclines): Move srcfile reading into read_srcfiles. > Add parameter to use cached srcfiles if available. > Also merge srcfiles with any files from DW_LNE_define_file. > (__libdw_getsrclines): Changed to call get_lines_or_files. > (__libdw_getsrcfiles): New function. Calls get_lines_or_files. > (get_lines_or_files): New function based on the old > __libdw_getsrclines. Call read_srcfiles if linesp is NULL, > otherwise call read_srclines. Pass previously read srcfiles > to read_srclines if available. > * libdw/dwarf_macro_getsrcfiles.c (dwarf_macro_getsrcfiles): > Replace __libdw_getsrclines with __libdw_getsrcfiles. > * libdw/libdwP.h (__libdw_getsrcfiles): New declaration. > * tests/.gitignore: Add new test binary. > * tests/get-files.c: Verify that dwarf_getsrcfiles does > not cause srclines to be read. > * tests/get-lines.c: Verify that srclines can be read > after srcfiles have been read. > * tests/Makefile.am: Add new testfiles. > * tests/get-files-define-file.c: Print file names before > and after reading DW_LNE_define_file. > * tests/run-get-files.sh: Add get-files-define-file test. > * tests/testfile-define-file.bz2: New testfile. Copy of > testfile36.debug but with a line program consisting of two > DW_LNE_define_file opcodes. >=20 > https://sourceware.org/bugzilla/show_bug.cgi?id=3D27405 >=20 > Signed-off-by: Aaron Merey > --- >=20 > v2 changes: > Restored support for DW_LNE_define_file. Great. And sorry I first suggested to just drop it and then said I would like it back. This was more work than I though. > Added DW_LNE_define_file testcase and testfile. Nice. How did you edit this file? > Added new function get_lines_or_files which is based on the old > __libdw_getsrclines. __libdw_getsrclines and __libdw_getsrcfiles now > call get_lines_or_files. It is just a simple rename, but so much nicer to my eyes. Thanks. > Mentioned max resident set size improvement in the commit message. I > did not mention a 5% speed up to rpm indexing that I have previously > talked about since I could not reliably reproduce it with v2 of the patch= . > I suspect that speed up may have been due to the smaller max RSS reducing > swapping during that round of testing. Makes sense. And a reduction of 75% of the max resident size it pretty huge in itself. > + case DW_LNE_define_file: > + { > + char *fname =3D (char *) linep; > + uint8_t *endp =3D memchr (linep, '\0', lineendp - linep)= ; > + if (endp =3D=3D NULL) > + goto invalid_data; > + size_t fnamelen =3D endp - linep; > + linep =3D endp + 1; > + > + unsigned int diridx; > + if (unlikely (linep >=3D lineendp)) > + goto invalid_data; > + get_uleb128 (diridx, linep, lineendp); > + > + size_t ndirs =3D (*filesp)->ndirs; This one line uses tabs for indentation, all others spaces. > + if (unlikely (diridx >=3D ndirs)) > + { > + __libdw_seterrno (DWARF_E_INVALID_DIR_IDX); > + goto invalid_data; > + } > + Dwarf_Word mtime; > + if (unlikely (linep >=3D lineendp)) > + goto invalid_data; > + get_uleb128 (mtime, linep, lineendp); > + Dwarf_Word filelength; > + if (unlikely (linep >=3D lineendp)) > + goto invalid_data; > + get_uleb128 (filelength, linep, lineendp); > + > + /* Add new_file to filelist that will be merged with filesp. */ > + struct filelist *new_file =3D malloc (sizeof (struct fil= elist)); > + if (unlikely (new_file =3D=3D NULL)) > { > - __libdw_seterrno (DWARF_E_INVALID_DIR_IDX); > - goto invalid_data; > + __libdw_seterrno (DWARF_E_NOMEM); > + goto out; > } And this last block uses tabs again. > - Dwarf_Word mtime; > - if (unlikely (linep >=3D lineendp)) > - goto invalid_data; > - get_uleb128 (mtime, linep, lineendp); > - Dwarf_Word filelength; > - if (unlikely (linep >=3D lineendp)) > - goto invalid_data; > - get_uleb128 (filelength, linep, lineendp); > - > - struct filelist *new_file =3D NEW_FILE (); > - if (fname[0] =3D=3D '/') > - new_file->info.name =3D fname; > - else > - { > - new_file->info.name =3D > - libdw_alloc (dbg, char, 1, (dirarray[diridx].len + 1 > - + fnamelen + 1)); > - char *cp =3D new_file->info.name; > - > - if (dirarray[diridx].dir !=3D NULL) > - /* This value could be NULL in case the > - DW_AT_comp_dir was not present. We > - cannot do much in this case. Just > - keep the file relative. */ > - { > - cp =3D stpcpy (cp, dirarray[diridx].dir); > - *cp++ =3D '/'; > - } > - strcpy (cp, fname); > - } > - > - new_file->info.mtime =3D mtime; > - new_file->info.length =3D filelength; > - } > - break; > + nfilelist++; > + new_file->next =3D filelist; > + filelist =3D new_file; Here tabs again, then the rest spaces. > + if (fname[0] =3D=3D '/') > + new_file->info.name =3D fname; > + else > + { > + /* Directory names are stored in a char *[ndirs] located > + after the last Dwarf_Fileinfo_s. */ > + size_t nfiles =3D (*filesp)->nfiles; > + const char **dirarray > + =3D (const char **) &((*filesp)->info[nfiles]); > + > + const char *dname =3D dirarray[diridx]; > + size_t dnamelen =3D strlen (dname); > + > + new_file->info.name =3D > + libdw_alloc (dbg, char, 1, (dnamelen + fnamelen + = 2)); > + char *cp =3D new_file->info.name; > + > + if (dname !=3D NULL) > + > + /* This value could be NULL in case the > + DW_AT_comp_dir was not present. We > + cannot do much in this case. Just > + keep the file relative. */ > + > + { > + cp =3D stpcpy (cp, dname); > + *cp++ =3D '/'; > + } > + strcpy (cp, fname); > + } > + > + new_file->info.mtime =3D mtime; > + new_file->info.length =3D filelength; > + } > + break; >=20 So inconsistent indentation, but the code looks good. > @@ -1027,32 +1183,49 @@ read_srclines (Dwarf *dbg, > } > } > =20 > - /* Put all the files in an array. */ > - Dwarf_Files *files =3D libdw_alloc (dbg, Dwarf_Files, > - sizeof (Dwarf_Files) > - + nfilelist * sizeof (Dwarf_Fileinfo) > - + (ndirlist + 1) * sizeof (char *), > - 1); > - const char **dirs =3D (void *) &files->info[nfilelist]; > - > - struct filelist *fileslist =3D filelist; > - files->nfiles =3D nfilelist; > - for (size_t n =3D nfilelist; n > 0; n--) > + /* Merge filesp with the files from DW_LNE_define_file, if any. */ > + if (unlikely (filelist !=3D NULL)) > { > - files->info[n - 1] =3D fileslist->info; > - fileslist =3D fileslist->next; > - } > - assert (fileslist =3D=3D NULL); > + Dwarf_Files *prevfiles =3D *filesp; > + size_t ndirs =3D prevfiles->ndirs; > + size_t nprevfiles =3D prevfiles->nfiles; > + size_t nnewfiles =3D nprevfiles + nfilelist; > + > + Dwarf_Files *newfiles > + =3D libdw_alloc (dbg, Dwarf_Files, > + sizeof (Dwarf_Files) > + + nnewfiles * sizeof (Dwarf_Fileinfo) > + + (ndirs + 1) * sizeof (char *), > + 1); > + > + > + /* Copy prevfiles to newfiles. */ > + for (size_t n =3D 0; n < nprevfiles; n++) > + newfiles->info[n] =3D prevfiles->info[n]; > + > + /* Add files from DW_LNE_define_file to newfiles. */ > + struct filelist *fileslist =3D filelist; > + for (size_t n =3D nfilelist; n > 0; n--) > + { > + newfiles->info[nprevfiles + n - 1] =3D fileslist->info; > + fileslist =3D fileslist->next; > + } > =20 > - /* Put all the directory strings in an array. */ > - files->ndirs =3D ndirlist; > - for (unsigned int i =3D 0; i < ndirlist; ++i) > - dirs[i] =3D dirarray[i].dir; > - dirs[ndirlist] =3D NULL; > + if (fileslist !=3D NULL) > + goto invalid_data; > =20 > - /* Pass the file data structure to the caller. */ > - if (filesp !=3D NULL) > - *filesp =3D files; > + const char **newdirs =3D (void *) &newfiles->info[nnewfiles]; > + const char **prevdirs =3D (void *) &prevfiles->info[nprevfiles]; > + > + /* Copy prevdirs to newdirs. */ > + for (size_t n =3D 0; n < ndirs; n++) > + newdirs[n] =3D prevdirs[n]; Again slightly off indentation. But I also don't fully follow the prevdirs/newdirs copying. Why is this? No newdirs are defined here, are there? Maybe I don't understand the data-structure used here. > diff --git a/tests/.gitignore b/tests/.gitignore > index 0289959d..e40ad238 100644 > --- a/tests/.gitignore > +++ b/tests/.gitignore > @@ -74,6 +74,7 @@ > /funcscopes > /get-aranges > /get-files > +/get-files-define_file > /get-lines > /get-pubnames > /get-units-invalid > diff --git a/tests/Makefile.am b/tests/Makefile.am > index 9141074f..7b016dc8 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -35,7 +35,7 @@ endif > check_PROGRAMS =3D arextract arsymtest newfile saridx scnnames sectiondu= mp \ > showptable update1 update2 update3 update4 test-nlist \ > show-die-info get-files next-files get-lines next-lines \ > - get-pubnames \ > + get-pubnames get-files-define-file \ > get-aranges allfcts line2addr addrscopes funcscopes \ > show-abbrev hash newscn ecp dwflmodtest \ > find-prologues funcretval allregs rdwrmmap \ > @@ -646,7 +646,8 @@ EXTRA_DIST =3D run-arextract.sh run-arsymtest.sh run-= ar.sh \ > testfile-dwp-5-cu-index-overflow.dwp.bz2 \ > testfile-dwp-4-cu-index-overflow.bz2 \ > testfile-dwp-4-cu-index-overflow.dwp.bz2 \ > - testfile-dwp-cu-index-overflow.source > + testfile-dwp-cu-index-overflow.source \ > + testfile-define-file.bz2 > =20 > =20 > if USE_VALGRIND > @@ -725,6 +726,7 @@ show_abbrev_LDADD =3D $(libdw) $(libelf) > get_lines_LDADD =3D $(libdw) $(libelf) > next_lines_LDADD =3D $(libdw) $(libelf) > get_files_LDADD =3D $(libdw) $(libelf) > +get_files_define_file_LDADD =3D $(libdw) $(libelf) > next_files_LDADD =3D $(libdw) $(libelf) > get_aranges_LDADD =3D $(libdw) $(libelf) > allfcts_LDADD =3D $(libdw) $(libelf) > diff --git a/tests/get-files-define-file.c b/tests/get-files-define-file.= c > new file mode 100644 > index 00000000..583f9852 > --- /dev/null > +++ b/tests/get-files-define-file.c > @@ -0,0 +1,162 @@ > +/* Copyright (C) 2002, 2004, 2005, 2007 Red Hat, Inc. > + This file is part of elfutils. > + Written by Ulrich Drepper , 2002. > + > + This file is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + elfutils is distributed in the hope that it will be useful, but > + WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see .= */ > + > +#ifdef HAVE_CONFIG_H > +# include > +#endif > + > +#include > +#include > +#include ELFUTILS_HEADER(dw) > +#include > +#include > +#include "../libdw/libdwP.h" > + > +static void > +print_dirs_and_files (Dwarf_Files *files, const char *const *dirs, > + size_t nfiles, size_t ndirs) > +{ > + if (dirs[0] =3D=3D NULL) > + puts (" dirs[0] =3D (null)"); > + else > + printf (" dirs[0] =3D \"%s\"\n", dirs[0]); > + for (size_t i =3D 1; i < ndirs; ++i) > + printf (" dirs[%zu] =3D \"%s\"\n", i, dirs[i]); > + > + for (size_t i =3D 0; i < nfiles; ++i) > + printf (" file[%zu] =3D \"%s\"\n", i, > + dwarf_filesrc (files, i, NULL, NULL)); > +} > + > + > +int > +main (int argc, char *argv[]) > +{ > + int result =3D 0; > + int cnt =3D argc - 1; > + > + int fd =3D open (argv[cnt], O_RDONLY); > + > + Dwarf *dbg =3D dwarf_begin (fd, DWARF_C_READ); > + if (dbg =3D=3D NULL) > + { > + printf ("%s not usable\n", argv[cnt]); > + result =3D 1; > + if (fd !=3D -1) > + close (fd); > + goto out; > + } > + > + Dwarf_Off o =3D 0; > + Dwarf_Off ncu; > + size_t cuhl; > + > + /* Just inspect the first CU. */ > + if (dwarf_nextcu (dbg, o, &ncu, &cuhl, NULL, NULL, NULL) !=3D 0) > + { > + printf ("%s: cannot get CU\n", argv[cnt]); > + result =3D 1; > + goto out; > + } > + > + Dwarf_Die die_mem; > + Dwarf_Die *die =3D dwarf_offdie (dbg, o + cuhl, &die_mem); > + > + if (die =3D=3D NULL) > + { > + printf ("%s: cannot get CU die\n", argv[cnt]); > + result =3D 1; > + goto out; > + } > + > + Dwarf_Files *files; > + size_t nfiles; > + > + /* The files from DW_LNE_define_file should not be included > + until dwarf_getsrclines is called. */ > + if (dwarf_getsrcfiles (die, &files, &nfiles) !=3D 0) > + { > + printf ("%s: cannot get files\n", argv[cnt]); > + result =3D 1; > + goto out; > + } > + > + if (die->cu->lines !=3D NULL) > + { > + printf ("%s: dwarf_getsrcfiles should not get lines\n", argv[cnt])= ; > + result =3D 1; > + goto out; > + } > + > + const char *const *dirs; > + size_t ndirs; > + if (dwarf_getsrcdirs (files, &dirs, &ndirs) !=3D 0) > + { > + printf ("%s: cannot get include directories\n", argv[cnt]); > + result =3D 1; > + goto out; > + } > + > + /* Print file info without files from DW_LNE_define_file. */ > + print_dirs_and_files (files, dirs, nfiles, ndirs); > + > + Dwarf_Lines *lines; > + size_t nlines; > + > + /* Reading the line program should add the new files. */ > + if (dwarf_getsrclines (die, &lines, &nlines) !=3D 0) > + { > + printf ("%s: cannot get lines\n", argv[cnt]); > + result =3D 1; > + goto out; > + } > + > + Dwarf_Files *updated_files; > + size_t num_updated_files; > + > + /* Get the new files. */ > + if (dwarf_getsrcfiles (die, &updated_files, &num_updated_files) !=3D 0= ) > + { > + printf ("%s: cannot get files\n", argv[cnt]); > + result =3D 1; > + goto out; > + } > + > + const char *const *updated_dirs; > + size_t num_updated_dirs; > + > + /* The dirs shouldn't change but verify that getsrcdirs still works. = */ > + if (dwarf_getsrcdirs (updated_files, &updated_dirs, &num_updated_dirs)= !=3D 0) > + { > + printf ("%s: cannot get include directories\n", argv[cnt]); > + result =3D 1; > + goto out; > + } > + > + /* Verify that we didn't invalidate the old file info. */ > + print_dirs_and_files (files, dirs, nfiles, ndirs); > + > + /* Print all files including those from DW_LNE_define_file. */ > + print_dirs_and_files (updated_files, updated_dirs, > + num_updated_files, num_updated_dirs); > + > +out: > + dwarf_end (dbg); > + close (fd); > + > + return result; > +} >=20 > [...] > diff --git a/tests/run-get-files.sh b/tests/run-get-files.sh > index 1306544d..c2bc01bf 100755 > --- a/tests/run-get-files.sh > +++ b/tests/run-get-files.sh > @@ -18,7 +18,7 @@ > =20 > . $srcdir/test-subr.sh > =20 > -testfiles testfile testfile2 > +testfiles testfile testfile2 testfile-define-file > =20 > testrun_compare ${abs_builddir}/get-files testfile testfile2 <<\EOF > cuhl =3D 11, o =3D 0, asz =3D 4, osz =3D 4, ncu =3D 191 > @@ -245,4 +245,67 @@ cuhl =3D 11, o =3D 0, asz =3D 8, osz =3D 4, ncu =3D = 857 > file[3] =3D "/usr/include/stdc-predef.h" > EOF > =20 > +tempfiles files define-files.out get-files-define-file.out > + > +cat > files <<\EOF > + dirs[0] =3D "session" > + dirs[1] =3D "/home/wcohen/minimal_mod" > + dirs[2] =3D "include/asm" > + dirs[3] =3D "include/linux" > + dirs[4] =3D "include/asm-generic" > + file[0] =3D "???" > + file[1] =3D "/home/wcohen/minimal_mod/minimal_mod.c" > + file[2] =3D "include/asm/gcc_intrin.h" > + file[3] =3D "include/linux/kernel.h" > + file[4] =3D "include/asm/processor.h" > + file[5] =3D "include/asm/types.h" > + file[6] =3D "include/asm/ptrace.h" > + file[7] =3D "include/linux/sched.h" > + file[8] =3D "include/asm/thread_info.h" > + file[9] =3D "include/linux/thread_info.h" > + file[10] =3D "include/asm/atomic.h" > + file[11] =3D "include/linux/list.h" > + file[12] =3D "include/linux/cpumask.h" > + file[13] =3D "include/linux/rbtree.h" > + file[14] =3D "include/asm/page.h" > + file[15] =3D "include/linux/rwsem.h" > + file[16] =3D "include/asm/rwsem.h" > + file[17] =3D "include/asm/spinlock.h" > + file[18] =3D "include/linux/completion.h" > + file[19] =3D "include/linux/wait.h" > + file[20] =3D "include/linux/aio.h" > + file[21] =3D "include/linux/workqueue.h" > + file[22] =3D "include/linux/timer.h" > + file[23] =3D "include/linux/types.h" > + file[24] =3D "include/asm/posix_types.h" > + file[25] =3D "include/linux/pid.h" > + file[26] =3D "include/linux/time.h" > + file[27] =3D "include/linux/capability.h" > + file[28] =3D "include/linux/signal.h" > + file[29] =3D "include/linux/resource.h" > + file[30] =3D "include/linux/sem.h" > + file[31] =3D "include/asm/fpu.h" > + file[32] =3D "include/linux/fs_struct.h" > + file[33] =3D "include/asm/signal.h" > + file[34] =3D "include/asm/siginfo.h" > + file[35] =3D "include/asm-generic/siginfo.h" > + file[36] =3D "include/asm/nodedata.h" > + file[37] =3D "include/linux/mmzone.h" > + file[38] =3D "include/linux/jiffies.h" > + file[39] =3D "include/asm/io.h" > + file[40] =3D "include/asm/machvec.h" > + file[41] =3D "include/asm/smp.h" > + file[42] =3D "include/asm/numa.h" > + file[43] =3D "include/linux/slab.h" > +EOF > + > +# Files should be printed 3 times, followed by the files from DW_LNE_def= ine_file > +cat files > define-files.out > +cat files >> define-files.out > +cat files >> define-files.out > +echo ' file[44] =3D "include/asm/abc.c"' >> define-files.out > +echo ' file[45] =3D "include/linux/01.c"' >> define-files.out > + > +cat define-files.out | testrun_compare ${abs_builddir}/get-files-define-= file testfile-define-file > + > exit 0 Nice. So testfile-define-file is actually testfile36.debug but with a new line program? How did you edit/insert that one? Cheers, Mark