Bug 232222 - rpm-4.4.2-debugpaths.patch broken
Summary: rpm-4.4.2-debugpaths.patch broken
Alias: None
Product: Fedora
Classification: Fedora
Component: rpm   
(Show other bugs)
Version: 6
Hardware: All Linux
Target Milestone: ---
Assignee: Paul Nasrat
QA Contact:
Keywords: Reopened
Depends On:
TreeView+ depends on / blocked
Reported: 2007-03-14 14:03 UTC by Jakub Jelinek
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2007-04-23 10:57:21 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)
rpm-4.4.2-debugedit-canonicalize-path.patch (1.37 KB, patch)
2007-03-14 14:47 UTC, Jakub Jelinek
no flags Details | Diff
rpm-debugedit-incremental-fix.patch (777 bytes, patch)
2007-04-20 21:12 UTC, Jakub Jelinek
no flags Details | Diff

Description Jakub Jelinek 2007-03-14 14:03:54 UTC
debugedit with rpm-4.4.2-debugpaths.patch breaks libgcj.so.8rh.0.0.debug.
Self-contained testcase:

rm -rf /tmp/debugedit-test
mkdir -p /tmp/debugedit-test/{a/b/c,d/e/f}
cd /tmp/debugedit-test/a/b/c
echo 'int main (void) { return 0; }' > ../../../d/e/f/a.c
gcc -g ../../../d/e/f/a.c -o a -O2
cd ../../../
readelf -wi -wl a/b/c/a | sed -n '/DW_AT_\(name.*\.c\|comp_dir\)/p;/^ The \(Directory\|File Name\) Table:/,/^$/p'
cp a/b/c/a{,.orig}
/usr/lib/rpm/debugedit -b /tmp/debugedit-test/ -d /usr/src/debug/ -l a/b/c/a.list a/b/c/a
readelf -wi -wl a/b/c/a | sed -n '/DW_AT_\(name.*\.c\|comp_dir\)/p;/^ The \(Directory\|File Name\) Table:/,/^$/p'

As you can see, it messes up the directory table entries, ../../../d/e/f
is changed into ../d/e/f and thus after relocation it obviously can't be found,
/usr/src/debug/a/b/c/../d/e/f/a.c is not where the file was stored -
/usr/src/debug/d/e/f/a.c aka /usr/src/debug/a/b/c/../../../d/e/f/a.c.

Comment 2 Jakub Jelinek 2007-03-14 14:47:03 UTC
Created attachment 150040 [details]

It seems canonicalize_path doesn't handle ../../ right (and messes other things
as well).
main (void)
  char buf[64];
  canonicalize_path("/a/b/././//c/../../d/e/", buf);
  __builtin_puts (buf);
  canonicalize_path("/a/b/././//c/..////../////d/e/", buf);
  __builtin_puts (buf);
  canonicalize_path("../a/b", buf);
  __builtin_puts (buf);
  canonicalize_path("../../a/b", buf);
  __builtin_puts (buf);
  canonicalize_path("../../../a/b", buf);
  __builtin_puts (buf);
  canonicalize_path("..///..///..///a/b", buf);
  __builtin_puts (buf);
  return 0;

with canonicalize_path from debugedit.c, it will print:

Note, ../../ was killed altogether, ../../../ shrunk into ../ and there are
several cases where multiple //'s are left.
With this patch it prints

Comment 3 Jeff Johnson 2007-03-15 14:22:32 UTC
Fixed in rpm cvs, will be in rpm-4.4.9-0.2 when built.

Comment 4 Paul Nasrat 2007-03-15 15:53:59 UTC
Thanks for the testcase (added to RHTS) and the patch.  Built for rawhide.

Do we need this for fc6 and RHEL5?  If so can you clone appropriately

Comment 5 Jakub Jelinek 2007-04-20 20:31:38 UTC
Unfortunately this patch causes regressions, e.g. glibc doesn't build any longer.
In particular, canonicalize_path on say "./netinet" canonicalizes it to
"/netinet" rather than "netinet".  Working on a fix.

Comment 6 Jakub Jelinek 2007-04-20 21:12:09 UTC
Created attachment 153216 [details]

Incremental patch to fix this bug.

Comment 8 Jeff Johnson 2007-04-20 23:48:36 UTC
Fixed in rpm cvs. Thanks for the patch.

(aside) I still think that path canonicalization on strings needs to be in glibc.
I personally have boogered up path canonicalization several times, clearly you have too ;-)

Comment 9 Jakub Jelinek 2007-04-21 06:58:21 UTC
Path canonicalization is in glibc, see realpath(3) and canonicalize_file_name(3).
But that doesn't simplify just ., .. and multiple //s, but also symlinks, so it
is not useful for debugedit purposes.

Comment 10 Jeff Johnson 2007-04-22 03:34:40 UTC
I knew realpath(3), whose behavior is dependent on the path existing on the file system
because of getcwd and stat.

A quick look at canonicalize_file_name indicates and alternative entry to __realpath, and
so has the same behavior. Thanks for the ptr.

A pure string canonicalization on path would be useful imho.

Note You need to log in before you can comment on or make changes to this bug.