Re: [Rpm-maint] [rpm-software-management/rpm] DWARF-5 support in debugedit (#1537)

[Mark Wielaard]

Hi Jan,

On Tue, 2021-02-16 at 01:37 -0800, Jan Kratochvil wrote:
> Could you rather point what was incomplete on the previous testcases
> than to screw it up? You removed the only part that matters.

I didn't screw up. I simply didn't use that part. But wrote some new
testcases based on the existing tests, just with explicit -gdwarf-4 and
-gdwarf-5 settings instead of tweaking the compiler used or needing new
tools in the tests. The reason I didn't use the testcases you wrote was
simply because they depended on llvm tooling being present and you said
you didn't care about making them work with gcc. I just happened to be
interested in testing with gcc, so I had to write some new testcases to
make sure the new functionality worked as expected.
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] DWARF-5 support in debugedit (#1537)

[Mark Wielaard]

Hi Jan,

On Tue, 2021-02-16 at 00:16 -0800, Jan Kratochvil wrote:
> The patchset regressed against my version as it no longer tests
> rpmbuild compatibility with LLVM product as required by paying
> customers of Red Hat company.

Unfortunately they did because I rewrote the tests so they didn't
require the llvm-toolset tools to run and made them independent of the
version of DWARF emitted by the compiler so that they work using either
older or newer gcc and binutils versions installed on the system.

Untested, but you can probably improve on that experience by changing
the gcc invocations inside the RPM_DEBUGEDIT_SETUP m4 macro (which is
just a shell script snippet) to use ${CC} instead. That should pick up
the CC setup by configure. That might also help people trying to build
and test rpm in a cross-compiler situation.

Cheers,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] debugedit: Distinguish files from directories in src list file (#1436)

[Mark Wielaard]

Hi,

Sorry for the late reply. Somehow the notification emails ended up in a
spam folder. sigh.

On Thu, 2020-12-10 at 04:29 -0800, Panu Matilainen wrote:
> Mark, any objections to this change? I like the idea of having
> directories explicitly marked with trailing slash, it goes with the
> spirit of what we're doing elsewhere already for a long time

I agree. The last commit 161c89 on pr/1436 looks good to me.

Cheers,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 1/5] [NFC] debugedit: Protect macro arguments by parentheses

[Mark Wielaard]

From: Jan Kratochvil 

---
 tools/debugedit.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/debugedit.c b/tools/debugedit.c
index e9d8f3ae7..87c1cd622 100644
--- a/tools/debugedit.c
+++ b/tools/debugedit.c
@@ -233,7 +233,7 @@ typedef struct
   int shift = 0;   \
   do   \
 {  \
-  c = *ptr++;  \
+  c = *(ptr)++;\
   ret |= (c & 0x7f) << shift;  \
   shift += 7;  \
 } while (c & 0x80);\
@@ -251,7 +251,7 @@ typedef struct
   valv >>= 7;  \
   if (valv)\
c |= 0x80;  \
-  *ptr++ = c;  \
+  *(ptr)++ = c;\
 }  \
   while (valv);\
 })
@@ -311,7 +311,7 @@ strptr (DSO *dso, int sec, off_t offset)
 }
 
 
-#define read_8(ptr) *ptr++
+#define read_8(ptr) *(ptr)++
 
 #define read_16(ptr) ({\
   uint16_t ret = do_read_16 (ptr); \
@@ -328,13 +328,13 @@ strptr (DSO *dso, int sec, off_t offset)
 REL *relptr, *relend;
 int reltype;
 
-#define do_read_32_relocated(ptr) ({   \
-  uint32_t dret = do_read_32 (ptr);\
+#define do_read_32_relocated(xptr) ({  \
+  uint32_t dret = do_read_32 (xptr);   \
   if (relptr)  \
 {  \
-  while (relptr < relend && relptr->ptr < ptr) \
+  while (relptr < relend && relptr->ptr < (xptr))  \
++relptr;   \
-  if (relptr < relend && relptr->ptr == ptr)   \
+  if (relptr < relend && relptr->ptr == (xptr))\
{   \
  if (reltype == SHT_REL)   \
dret += relptr->addend; \
-- 
2.18.4

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 2/5] [NFC] debugedit: Move code to separate functions.

[Mark Wielaard]

From: Jan Kratochvil 

New functions edit_strp, skip_form and edit_attributes_str_comp_dir
called by edit_attributes.
Split part of read_dwarf2_line into a read_dwarf4_line function.
New function edit_dwarf2_any_str called by edit_dwarf2 at the end of
phase 0 to rebuild .debug_str.
---
 tools/debugedit.c | 367 ++
 1 file changed, 212 insertions(+), 155 deletions(-)

diff --git a/tools/debugedit.c b/tools/debugedit.c
index 87c1cd622..7464883c5 100644
--- a/tools/debugedit.c
+++ b/tools/debugedit.c
@@ -1457,37 +1457,128 @@ edit_dwarf2_line (DSO *dso)
 }
 }
 
-/* Called during phase zero for each debug_line table referenced from
-   .debug_info.  Outputs all source files seen and records any
-   adjustments needed in the debug_list data structures. Returns true
-   if line_table needs to be rewrite either the dir or file paths. */
+/* Record or adjust (according to phase) DW_FORM_strp.  */
+static void
+edit_strp (DSO *dso, unsigned char *ptr, int phase, bool handled_strp)
+{
+  unsigned char *ptr_orig = ptr;
+
+  /* In the first pass we collect all strings, in the
+ second we put the new references back (if there are
+ any changes).  */
+  if (phase == 0)
+{
+  /* handled_strp is set for attributes referring to
+files. If it is set the string is already
+recorded. */
+  if (! handled_strp)
+   {
+ size_t idx = do_read_32_relocated (ptr);
+ record_existing_string_entry_idx (>strings, idx);
+   }
+}
+  else if (need_strp_update) /* && phase == 1 */
+{
+  struct stridxentry *entry;
+  size_t idx, new_idx;
+  idx = do_read_32_relocated (ptr);
+  entry = string_find_entry (>strings, idx);
+  new_idx = strent_offset (entry->entry);
+  do_write_32_relocated (ptr, new_idx);
+}
+
+  assert (ptr == ptr_orig);
+}
+
+/* Adjust *PTRP after the current *FORMP, update *FORMP for FORM_INDIRECT.  */
+static enum { FORM_OK, FORM_ERROR, FORM_INDIRECT }
+skip_form (DSO *dso, uint32_t *formp, unsigned char **ptrp)
+{
+  size_t len = 0;
+
+  switch (*formp)
+{
+case DW_FORM_ref_addr:
+  if (cu_version == 2)
+   *ptrp += ptr_size;
+  else
+   *ptrp += 4;
+  break;
+case DW_FORM_flag_present:
+  break;
+case DW_FORM_addr:
+  *ptrp += ptr_size;
+  break;
+case DW_FORM_ref1:
+case DW_FORM_flag:
+case DW_FORM_data1:
+  ++*ptrp;
+  break;
+case DW_FORM_ref2:
+case DW_FORM_data2:
+  *ptrp += 2;
+  break;
+case DW_FORM_ref4:
+case DW_FORM_data4:
+case DW_FORM_sec_offset:
+  *ptrp += 4;
+  break;
+case DW_FORM_ref8:
+case DW_FORM_data8:
+case DW_FORM_ref_sig8:
+  *ptrp += 8;
+  break;
+case DW_FORM_sdata:
+case DW_FORM_ref_udata:
+case DW_FORM_udata:
+  read_uleb128 (*ptrp);
+  break;
+case DW_FORM_strp:
+  *ptrp += 4;
+  break;
+case DW_FORM_string:
+  *ptrp = (unsigned char *) strchr ((char *)*ptrp, '\0') + 1;
+  break;
+case DW_FORM_indirect:
+  *formp = read_uleb128 (*ptrp);
+  return FORM_INDIRECT;
+case DW_FORM_block1:
+  len = *(*ptrp)++;
+  break;
+case DW_FORM_block2:
+  len = read_16 (*ptrp);
+  *formp = DW_FORM_block1;
+  break;
+case DW_FORM_block4:
+  len = read_32 (*ptrp);
+  *formp = DW_FORM_block1;
+  break;
+case DW_FORM_block:
+case DW_FORM_exprloc:
+  len = read_uleb128 (*ptrp);
+  *formp = DW_FORM_block1;
+  assert (len < UINT_MAX);
+  break;
+default:
+  error (0, 0, "%s: Unknown DWARF DW_FORM_%d", dso->filename, *formp);
+  return FORM_ERROR;
+}
+
+  if (*formp == DW_FORM_block1)
+*ptrp += len;
+
+  return FORM_OK;
+}
+
+/* Part of read_dwarf2_line processing DWARF-4.  */
 static bool
-read_dwarf2_line (DSO *dso, uint32_t off, char *comp_dir)
+read_dwarf4_line (DSO *dso, unsigned char *ptr, char *comp_dir,
+ struct line_table *table)
 {
-  unsigned char *ptr, *dir;
   unsigned char **dirt;
   uint32_t value, dirt_cnt;
   size_t comp_dir_len = !comp_dir ? 0 : strlen (comp_dir);
-  struct line_table *table;
-
-  if (get_line_table (dso, off, ) == false
-  || table == NULL)
-return false;
-
-  /* Skip to the directory table. The rest of the header has already
- been read and checked by get_line_table. */
-  ptr = debug_sections[DEBUG_LINE].data + off;
-  ptr += (4 /* unit len */
- + 2 /* version */
- + 4 /* header len */
- + 1 /* min instr len */
- + (table->version >= 4) /* max op per instr, if version >= 4 */
- + 1 /* default is stmt */
- + 1 /* line base */
- + 1 /* line range */
- + 1 /* opcode base */
- + table->opcode_base - 1); /* opcode len table */
-  dir = ptr;
+  unsigned char *dir = ptr;
 
   /* dir table: */
   value = 1;
@@ -1620,6 +1711,40 @@ read_dwarf2_line (DSO *dso, uint32_t off, char *comp_dir)
   

[Rpm-maint] debugedit: Implement DWARF-5. (#1332)

[Mark Wielaard]

Hi,

It looks like the rpm-maint github bridge doesn't always work flawlessly
with attached patches. So here the whole series is using normal git
send-email. They are also available at:
https://code.wildebeest.org/git/user/mjw/rpm/log/?h=gcc-dwarf5

These patches are based on the original patches by Jan Kratochvil
but rewritten a bit so each individual patch adds distinct new
functionality and adding explicit DWARF4 and DWARF5 testcases using
the default GCC compiler that should work whether or not the compiler
defaults to DWARF4 or DWARF5 and whether or not the binutils assembler
generates older or new .debug_line sections. The patches (except the
new test cases) have been tested during the recent Fedora mass-rebuild
that used a GCC11 pre-release that defaulted to DWARF5 by default
using binutils 2.35.2. This resulted in one bug fix, a confusion of
comparing the size of the .debug_str and the new .debug_line_str section.
Those patches were against the rpm 4.16.1.2 release and can be found here:
https://code.wildebeest.org/git/user/mjw/rpm/log/?h=gcc-dwarf5-4.16.1.2

Those 4.16.1.2 patches include two patches which were already applied
to master:
- [NFC] debugedit: Move code from edit_dwarf2() to edit_info().
- debugedit: Fix missing relocation of .debug_types section.
So those have been dropped in this series, which was rebased to
current git trunk:

 [PATCH 1/5] [NFC] debugedit: Protect macro arguments by parentheses
 [PATCH 2/5] [NFC] debugedit: Move code to separate functions.
 [PATCH 3/5] debugedit: Implement DWARF-5 unit header and new forms
 [PATCH 4/5] debugedit: Handle DWARF-5 debug_line and debug_line_str.
 [PATCH 5/5] debugedit: Add DWARF5 tests

The first two [NFC] patches are unchanged just have a bit more explanation
in the commit message about what is going on.

The original series contained one patch that was dropped:
- debugedit: Fix handling in caller for errors in called read_dwarf2_line.
It was needed when doing multiple passes of some phases when dealing
with .debug_line, but the new patch handles things by doing each phase
just once (that is the fourth patch).

The original patches:
- debugedit: Implement DWARF-5 .debug_types (in .debug_info).
- debugedit: Implement DWARF-5.
were rewritten and split into the last three patches to have separate
implementations of handling the new unit headers and forms, handling
of the new .debug_line and .debug_line_str and adding new test cases.
This also made the third patch testable independently with older GCC
versions that support -gdwarf-5 but don't yet generate DWARF5 debug
line tables.

One feature of the original patch was dropped, handling of DW_UT_types
in .debug_info, which seemed to complicate the code a bit and wasn't
necessary for the default GCC DWARF5 mode. It shouldn't be that hard
to add that back if needed (although handling the partially linked
variant might be slightly tricky since they require handling COMDAT
.debug_info sections).

The original series added tests based on llvm tooling, which I have
replaced by existing tests based on explicit -gdwarf-4 and -gdwarf-5
tests that should work fine against both older and newer gcc versions
which might have different DWARF version defaults.

Cheers,

Mark

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 4/5] debugedit: Handle DWARF-5 debug_line and debug_line_str.

[Mark Wielaard]

Handle the new DWARF5 .debug_line tables and the new DW_FORM_line_strp.
DWARF5 tables are handled separately from the earlier tables. They
will never change size, but they do need updates to the .debug_str
or .debug_line_str references.

Based on a patch from Jan Kratochvil 
---
 tools/debugedit.c | 471 --
 1 file changed, 410 insertions(+), 61 deletions(-)

diff --git a/tools/debugedit.c b/tools/debugedit.c
index be5fee85b..b0849efd8 100644
--- a/tools/debugedit.c
+++ b/tools/debugedit.c
@@ -103,6 +103,8 @@ static bool need_string_replacement = false;
 /* Whether we need to do any updates of the string indexes (DW_FORM_strp)
in debug_info for string indexes. */
 static bool need_strp_update = false;
+/* Likewise for DW_FORM_line_strp. */
+static bool need_line_strp_update = false;
 /* If the debug_line changes size we will need to update the
DW_AT_stmt_list attributes indexes in the debug_info. */
 static bool need_stmt_update = false;
@@ -192,7 +194,7 @@ typedef struct
   const char *filename;
   int lastscn;
   size_t phnum;
-  struct strings strings;
+  struct strings debug_str, debug_line_str;
   struct debug_lines lines;
   GElf_Shdr shdr[0];
 } DSO;
@@ -553,10 +555,11 @@ setup_relbuf (DSO *dso, debug_section *sec, int *reltype)
   /* Relocations against section symbols are uninteresting in REL.  */
   if (dso->shdr[i].sh_type == SHT_REL && sym.st_value == 0)
continue;
-  /* Only consider relocations against .debug_str, .debug_line
-and .debug_abbrev.  */
+  /* Only consider relocations against .debug_str, .debug_line,
+.debug_line_str, and .debug_abbrev.  */
   if (sym.st_shndx != debug_sections[DEBUG_STR].sec
  && sym.st_shndx != debug_sections[DEBUG_LINE].sec
+ && sym.st_shndx != debug_sections[DEBUG_LINE_STR].sec
  && sym.st_shndx != debug_sections[DEBUG_ABBREV].sec)
continue;
   rela.r_addend += sym.st_value;
@@ -768,6 +771,7 @@ no_memory:
  || (form > DW_FORM_flag_present
  && !(form == DW_FORM_ref_sig8
   || form == DW_FORM_data16
+  || form == DW_FORM_line_strp
   || form == DW_FORM_implicit_const
   || form == DW_FORM_addrx
   || form == DW_FORM_loclistx
@@ -1049,17 +1053,20 @@ string_find_entry (struct strings *strings, size_t 
old_idx)
a replacement file string has been recorded for it, otherwise
returns false.  */
 static bool
-record_file_string_entry_idx (struct strings *strings, size_t old_idx)
+record_file_string_entry_idx (bool line_strp, DSO *dso, size_t old_idx)
 {
+  struct strings *strings = line_strp ? >debug_line_str : >debug_str;
   bool ret = false;
   struct stridxentry *entry = string_find_new_entry (strings, old_idx);
   if (entry != NULL)
 {
-  if (old_idx >= debug_sections[DEBUG_STR].size)
-   error (1, 0, "Bad string pointer index %zd", old_idx);
+  debug_section *sec = _sections[line_strp
+  ? DEBUG_LINE_STR : DEBUG_STR];
+  if (old_idx >= sec->size)
+   error (1, 0, "Bad string pointer index %zd (%s)", old_idx, sec->name);
 
   Strent *strent;
-  const char *old_str = (char *)debug_sections[DEBUG_STR].data + old_idx;
+  const char *old_str = (char *)sec->data + old_idx;
   const char *file = skip_dir_prefix (old_str, base_dir);
   if (file == NULL)
{
@@ -1103,15 +1110,18 @@ record_file_string_entry_idx (struct strings *strings, 
size_t old_idx)
base_dir with dest_dir, just records the existing string associated
with the index. */
 static void
-record_existing_string_entry_idx (struct strings *strings, size_t old_idx)
+record_existing_string_entry_idx (bool line_strp, DSO *dso, size_t old_idx)
 {
+  struct strings *strings = line_strp ? >debug_line_str : >debug_str;
   struct stridxentry *entry = string_find_new_entry (strings, old_idx);
   if (entry != NULL)
 {
-  if (old_idx >= debug_sections[DEBUG_STR].size)
-   error (1, 0, "Bad string pointer index %zd", old_idx);
+  debug_section *sec = _sections[line_strp
+  ? DEBUG_LINE_STR : DEBUG_STR];
+  if (old_idx >= sec->size)
+   error (1, 0, "Bad string pointer index %zd (%s)", old_idx, sec->name);
 
-  const char *str = (char *)debug_sections[DEBUG_STR].data + old_idx;
+  const char *str = (char *)sec->data + old_idx;
   Strent *strent = strtab_add_len (strings->str_tab,
   str, strlen (str) + 1);
   if (strent == NULL)
@@ -1244,13 +1254,28 @@ get_line_table (DSO *dso, size_t off, struct line_table 
**table)
 
   /* version */
   t->version = read_16 (ptr);
-  if (t->version != 2 && t->version != 3 && t->version != 4)
+  if (t->version != 2 && t->version != 3 && t->version != 4 && t->version != 5)
 {
   error (0, 0, "%s: DWARF version 

[Rpm-maint] [PATCH 5/5] debugedit: Add DWARF5 tests

[Mark Wielaard]

Adjust some existing tests so they are run with an explicit -gdwarf-4
and add an -gdwarf-5 variant to make testing independent of the gcc
default DWARF version. The tests that might generate a DWARF5 line
table work for both version 4 and 5 and some also ignore stderr output
when using -p.debug_line_str because some combinations of gcc/binutils
generate DWARF5 debug info with DWARF4 debug line tables, even when
-gdwarf-5 is given.

Tested against GCC10, which defaults to DWARF4 and GCC11, which defaults
to DWARF5.
---
 tests/debugedit.at | 195 +
 1 file changed, 180 insertions(+), 15 deletions(-)

diff --git a/tests/debugedit.at b/tests/debugedit.at
index 49721a342..a78180e13 100644
--- a/tests/debugedit.at
+++ b/tests/debugedit.at
@@ -77,9 +77,9 @@ AT_CLEANUP
 # debugedit should at least replace the .debug_str directory paths
 # in the objects.
 # ===
-AT_SETUP([debugedit .debug_str objects])
+AT_SETUP([debugedit .debug_str objects DWARF4])
 AT_KEYWORDS([debuginfo] [debugedit])
-RPM_DEBUGEDIT_SETUP
+RPM_DEBUGEDIT_SETUP([-gdwarf-4])
 
 # Capture strings that start with the testdir (pwd) directory path
 # (and replace that textually with /foo/bar/baz)
@@ -104,13 +104,46 @@ readelf -p.debug_str foo.o subdir_bar/bar.o baz.o | cut 
-c13- \
 
 AT_CLEANUP
 
+# ===
+# debugedit should at least replace the .debug_str/line_str directory paths
+# in the objects.
+# ===
+AT_SETUP([debugedit .debug_str/line_str objects DWARF5])
+AT_KEYWORDS([debuginfo] [debugedit])
+RPM_DEBUGEDIT_SETUP([-gdwarf-5])
+
+# Capture strings that start with the testdir (pwd) directory path
+# (and replace that textually with /foo/bar/baz)
+readelf -p.debug_str -p.debug_line_str foo.o subdir_bar/bar.o baz.o \
+| cut -c13- \
+| grep ^$(pwd) | sort | uniq \
+| sed -e "s@$(pwd)@/foo/bar/baz@" > expout
+
+# Make sure there is at least some output
+expout_lines=$(wc --lines expout | cut -f1 -d\ )
+if test $expout_lines -lt 3; then
+  echo "Expecting at least 3 debug strings starting with ${testdir}" >> expout
+fi
+
+# Check the replaced strings are all there.
+AT_CHECK([[debugedit -b $(pwd) -d /foo/bar/baz ./foo.o]])
+AT_CHECK([[debugedit -b $(pwd) -d /foo/bar/baz ./subdir_bar/bar.o]])
+AT_CHECK([[debugedit -b $(pwd) -d /foo/bar/baz ./baz.o]])
+AT_CHECK([[
+readelf -p.debug_str -p.debug_line_str foo.o subdir_bar/bar.o baz.o \
+| cut -c13- \
+| grep ^/foo/bar/baz | sort | uniq
+]],[0],[expout],[ignore])
+
+AT_CLEANUP
+
 # ===
 # debugedit should at least replace the .debug_str directory paths
 # also in partially linked files.
 # ===
-AT_SETUP([debugedit .debug_str partial])
+AT_SETUP([debugedit .debug_str partial DWARF4])
 AT_KEYWORDS([debuginfo] [debugedit])
-RPM_DEBUGEDIT_SETUP
+RPM_DEBUGEDIT_SETUP([-gdwarf-4])
 
 # Capture strings that start with the testdir (pwd) directory path
 # (and replace that textually with /foo/bar/baz)
@@ -135,13 +168,44 @@ readelf -p.debug_str ./foobarbaz.part.o | cut -c13- \
 
 AT_CLEANUP
 
+# ===
+# debugedit should at least replace the .debug_str/line_str directory paths
+# also in partially linked files.
+# ===
+AT_SETUP([debugedit .debug_str/line_str partial DWARF5])
+AT_KEYWORDS([debuginfo] [debugedit])
+RPM_DEBUGEDIT_SETUP([-gdwarf-5])
+
+# Capture strings that start with the testdir (pwd) directory path
+# (and replace that textually with /foo/bar/baz)
+# Note that partially linked files, might have multiple duplicate
+# strings, but debugedit will merge them. So use sort -u.
+readelf -p.debug_str -p.debug_line_str ./foobarbaz.part.o | cut -c13- \
+| grep ^$(pwd) | sort -u | uniq \
+| sed -e "s@$(pwd)@/foo/bar/baz@" > expout
+
+# Make sure there is at least some output
+expout_lines=$(wc --lines expout | cut -f1 -d\ )
+if test $expout_lines -lt 3; then
+  echo "Expecting at least 3 debug strings starting with ${testdir}" >> expout
+fi
+
+# Check the replaced strings are all there.
+AT_CHECK([[debugedit -b $(pwd) -d /foo/bar/baz ./foobarbaz.part.o]])
+AT_CHECK([[
+readelf -p.debug_str -p.debug_line_str ./foobarbaz.part.o | cut -c13- \
+| grep ^/foo/bar/baz | sort | uniq
+]],[0],[expout],[ignore])
+
+AT_CLEANUP
+
 # ===
 # debugedit should at least replace the .debug_str directory paths
 # and in the executable.
 # ===
-AT_SETUP([debugedit .debug_str exe])
+AT_SETUP([debugedit .debug_str exe DWARF4])
 AT_KEYWORDS([debuginfo] [debugedit])
-RPM_DEBUGEDIT_SETUP
+RPM_DEBUGEDIT_SETUP([-gdwarf-4])
 
 # Capture strings that start with the testdir (pwd) directory path
 # (and replace that textually with /foo/bar/baz)
@@ -165,6 +229,36 @@ readelf -p.debug_str foobarbaz.exe | cut -c13- \
 
 AT_CLEANUP
 
+# ===
+# debugedit should at least replace the .debug_str/line_str directory paths
+# and in the executable.
+# ===
+AT_SETUP([debugedit .debug_str/line_str exe DWARF5])
+AT_KEYWORDS([debuginfo] [debugedit])
+RPM_DEBUGEDIT_SETUP([-gdwarf-5])
+
+# Capture strings that start with the testdir (pwd) directory path
+# 

[Rpm-maint] [PATCH 3/5] debugedit: Implement DWARF-5 unit header and new forms parsing.

[Mark Wielaard]

From: Jan Kratochvil 

Recognize the various new DWARF5 .debug sections.
Parse and skip new DWARF5 forms in read_abbrev and skip_form.
Read DWARF5 unit headers for compile and partial units in edit_info.

This is enough to be able to process gcc -gdwarf-5 produced binaries
without the new DWARF5 .debug_line format (which isn't produced with
binutils < 2.36).

Patches slightly edited/merged by Mark Wielaard 
---
 tools/debugedit.c | 88 +++
 1 file changed, 81 insertions(+), 7 deletions(-)

diff --git a/tools/debugedit.c b/tools/debugedit.c
index 7464883c5..be5fee85b 100644
--- a/tools/debugedit.c
+++ b/tools/debugedit.c
@@ -453,6 +453,11 @@ static debug_section debug_sections[] =
 #define DEBUG_TYPES11
 #define DEBUG_MACRO12
 #define DEBUG_GDB_SCRIPT   13
+#define DEBUG_RNGLISTS 14
+#define DEBUG_LINE_STR 15
+#define DEBUG_ADDR 16
+#define DEBUG_STR_OFFSETS  17
+#define DEBUG_LOCLISTS 18
 { ".debug_info", NULL, NULL, 0, 0, 0 },
 { ".debug_abbrev", NULL, NULL, 0, 0, 0 },
 { ".debug_line", NULL, NULL, 0, 0, 0 },
@@ -467,6 +472,11 @@ static debug_section debug_sections[] =
 { ".debug_types", NULL, NULL, 0, 0, 0 },
 { ".debug_macro", NULL, NULL, 0, 0, 0 },
 { ".debug_gdb_scripts", NULL, NULL, 0, 0, 0 },
+{ ".debug_rnglists", NULL, NULL, 0, 0, 0 },
+{ ".debug_line_str", NULL, NULL, 0, 0, 0 },
+{ ".debug_addr", NULL, NULL, 0, 0, 0 },
+{ ".debug_str_offsets", NULL, NULL, 0, 0, 0 },
+{ ".debug_loclists", NULL, NULL, 0, 0, 0 },
 { NULL, NULL, NULL, 0, 0, 0 }
   };
 
@@ -755,12 +765,28 @@ no_memory:
}
  form = read_uleb128 (ptr);
  if (form == 2
- || (form > DW_FORM_flag_present && form != DW_FORM_ref_sig8))
+ || (form > DW_FORM_flag_present
+ && !(form == DW_FORM_ref_sig8
+  || form == DW_FORM_data16
+  || form == DW_FORM_implicit_const
+  || form == DW_FORM_addrx
+  || form == DW_FORM_loclistx
+  || form == DW_FORM_rnglistx
+  || form == DW_FORM_addrx1
+  || form == DW_FORM_addrx2
+  || form == DW_FORM_addrx3
+  || form == DW_FORM_addrx4)))
{
- error (0, 0, "%s: Unknown DWARF DW_FORM_%d", dso->filename, form);
+ error (0, 0, "%s: Unknown DWARF DW_FORM_0x%x", dso->filename,
+form);
  htab_delete (h);
  return NULL;
}
+ if (form == DW_FORM_implicit_const)
+   {
+ /* It is SLEB128 but the value is dropped anyway.  */
+ read_uleb128 (ptr);
+   }
 
  t->attr[t->nattr].attr = attr;
  t->attr[t->nattr++].form = form;
@@ -1505,6 +1531,7 @@ skip_form (DSO *dso, uint32_t *formp, unsigned char 
**ptrp)
*ptrp += 4;
   break;
 case DW_FORM_flag_present:
+case DW_FORM_implicit_const:
   break;
 case DW_FORM_addr:
   *ptrp += ptr_size;
@@ -1512,14 +1539,24 @@ skip_form (DSO *dso, uint32_t *formp, unsigned char 
**ptrp)
 case DW_FORM_ref1:
 case DW_FORM_flag:
 case DW_FORM_data1:
+case DW_FORM_strx1:
+case DW_FORM_addrx1:
   ++*ptrp;
   break;
 case DW_FORM_ref2:
 case DW_FORM_data2:
+case DW_FORM_strx2:
+case DW_FORM_addrx2:
   *ptrp += 2;
   break;
+case DW_FORM_strx3:
+case DW_FORM_addrx3:
+  *ptrp += 3;
+  break;
 case DW_FORM_ref4:
 case DW_FORM_data4:
+case DW_FORM_strx4:
+case DW_FORM_addrx4:
 case DW_FORM_sec_offset:
   *ptrp += 4;
   break;
@@ -1528,12 +1565,20 @@ skip_form (DSO *dso, uint32_t *formp, unsigned char 
**ptrp)
 case DW_FORM_ref_sig8:
   *ptrp += 8;
   break;
+case DW_FORM_data16:
+  *ptrp += 16;
+  break;
 case DW_FORM_sdata:
 case DW_FORM_ref_udata:
 case DW_FORM_udata:
+case DW_FORM_strx:
+case DW_FORM_loclistx:
+case DW_FORM_rnglistx:
+case DW_FORM_addrx:
   read_uleb128 (*ptrp);
   break;
 case DW_FORM_strp:
+case DW_FORM_line_strp:
   *ptrp += 4;
   break;
 case DW_FORM_string:
@@ -1560,7 +1605,7 @@ skip_form (DSO *dso, uint32_t *formp, unsigned char 
**ptrp)
   assert (len < UINT_MAX);
   break;
 default:
-  error (0, 0, "%s: Unknown DWARF DW_FORM_%d", dso->filename, *formp);
+  error (0, 0, "%s: Unknown DWARF DW_FORM_0x%x", dso->filename, *formp);
   return FORM_ERROR;
 }
 
@@ -2030,7 +2075,10 @@ edit_info (DSO *dso, int phase, struct debug_section 
*sec)
   endsec = ptr + sec->size;
   while (ptr < endsec)
 {
-  if (ptr + (sec == _sections[DEBUG_

Re: [Rpm-maint] [rpm-software-management/rpm] debugedit: Do not 'edit_dwarf2' when just extracting build-id (#1435)

[Mark Wielaard]

On Sun, 2020-11-15 at 12:18 -0800, Vitalio wrote:
> > Yes, but you also have to check base_dir && dest_dir aren't set.
> 
> I will rework.

You last patch looks good.

Thanks,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] debugedit: Distinguish files from directories in src list file (#1436)

[Mark Wielaard]

Hi Vitalio,

On Sun, 2020-11-15 at 12:35 -0800, Vitalio wrote:
> BTW. How does this help you speed things up?
> 
> We do just `grep -v '/$' to filter directories from source list[1], instead 
> of loop similar to `while read f; do test -d $f || echo $f; done` which takes 
> more than 15 seconds on kernel package.
> 
>  [1] 
> http://git.altlinux.org/people/vt/packages/?p=rpm-build.git;a=commitdiff;h=623707e72815

aha, you have a separate process-debuginfo and find-debuginfo-files
script that I assume replaces the upstream find-debuginfo.sh script.
Yours seem much simpler than what we have in find-debuginfo.sh, which
has grown support for dwz, mini-symtab, etc. Might it be an idea to
split find-debuginfo.sh upstream and adopt/merge your scripts?

BTW. I gave you bad advice, p[0] is after the last character written,
so will always be the zero string terminator because we now use
strlen() - 1. It should have been *(p-1) != '/'. Which should always
work because we filter out zero length strings in the if statement
before the block, so there is always  at least one (last) character.
Sorry about that.

Cheers,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] [NFC] debugedit: Move code to separate functions.

[Mark Wielaard]

Hi,

This looks OK. I added a description of which functions were separate
from where to the commit message and rebased the commit to the master
branch.

Cheers,

Mark
From ca137a2fa64eacfd0ad42b89c8436d78e1143820 Mon Sep 17 00:00:00 2001
From: Jan Kratochvil 
Date: Mon, 17 Aug 2020 21:58:19 +0200
Subject: [PATCH] [NFC] debugedit: Move code to separate functions.

New functions edit_strp, skip_form and edit_attributes_str_comp_dir
called by edit_attributes.
Split part of read_dwarf2_line into a read_dwarf4_line function.
New function edit_dwarf2_any_str called by edit_dwarf2 at the end of
phase 0 to rebuild .debug_str.
---
 tools/debugedit.c | 367 ++
 1 file changed, 212 insertions(+), 155 deletions(-)

diff --git a/tools/debugedit.c b/tools/debugedit.c
index 8e85847d1..92509ae78 100644
--- a/tools/debugedit.c
+++ b/tools/debugedit.c
@@ -1457,37 +1457,128 @@ edit_dwarf2_line (DSO *dso)
 }
 }
 
-/* Called during phase zero for each debug_line table referenced from
-   .debug_info.  Outputs all source files seen and records any
-   adjustments needed in the debug_list data structures. Returns true
-   if line_table needs to be rewrite either the dir or file paths. */
+/* Record or adjust (according to phase) DW_FORM_strp.  */
+static void
+edit_strp (DSO *dso, unsigned char *ptr, int phase, bool handled_strp)
+{
+  unsigned char *ptr_orig = ptr;
+
+  /* In the first pass we collect all strings, in the
+ second we put the new references back (if there are
+ any changes).  */
+  if (phase == 0)
+{
+  /* handled_strp is set for attributes referring to
+	 files. If it is set the string is already
+	 recorded. */
+  if (! handled_strp)
+	{
+	  size_t idx = do_read_32_relocated (ptr);
+	  record_existing_string_entry_idx (>strings, idx);
+	}
+}
+  else if (need_strp_update) /* && phase == 1 */
+{
+  struct stridxentry *entry;
+  size_t idx, new_idx;
+  idx = do_read_32_relocated (ptr);
+  entry = string_find_entry (>strings, idx);
+  new_idx = strent_offset (entry->entry);
+  do_write_32_relocated (ptr, new_idx);
+}
+
+  assert (ptr == ptr_orig);
+}
+
+/* Adjust *PTRP after the current *FORMP, update *FORMP for FORM_INDIRECT.  */
+static enum { FORM_OK, FORM_ERROR, FORM_INDIRECT }
+skip_form (DSO *dso, uint32_t *formp, unsigned char **ptrp)
+{
+  size_t len = 0;
+
+  switch (*formp)
+{
+case DW_FORM_ref_addr:
+  if (cu_version == 2)
+	*ptrp += ptr_size;
+  else
+	*ptrp += 4;
+  break;
+case DW_FORM_flag_present:
+  break;
+case DW_FORM_addr:
+  *ptrp += ptr_size;
+  break;
+case DW_FORM_ref1:
+case DW_FORM_flag:
+case DW_FORM_data1:
+  ++*ptrp;
+  break;
+case DW_FORM_ref2:
+case DW_FORM_data2:
+  *ptrp += 2;
+  break;
+case DW_FORM_ref4:
+case DW_FORM_data4:
+case DW_FORM_sec_offset:
+  *ptrp += 4;
+  break;
+case DW_FORM_ref8:
+case DW_FORM_data8:
+case DW_FORM_ref_sig8:
+  *ptrp += 8;
+  break;
+case DW_FORM_sdata:
+case DW_FORM_ref_udata:
+case DW_FORM_udata:
+  read_uleb128 (*ptrp);
+  break;
+case DW_FORM_strp:
+  *ptrp += 4;
+  break;
+case DW_FORM_string:
+  *ptrp = (unsigned char *) strchr ((char *)*ptrp, '\0') + 1;
+  break;
+case DW_FORM_indirect:
+  *formp = read_uleb128 (*ptrp);
+  return FORM_INDIRECT;
+case DW_FORM_block1:
+  len = *(*ptrp)++;
+  break;
+case DW_FORM_block2:
+  len = read_16 (*ptrp);
+  *formp = DW_FORM_block1;
+  break;
+case DW_FORM_block4:
+  len = read_32 (*ptrp);
+  *formp = DW_FORM_block1;
+  break;
+case DW_FORM_block:
+case DW_FORM_exprloc:
+  len = read_uleb128 (*ptrp);
+  *formp = DW_FORM_block1;
+  assert (len < UINT_MAX);
+  break;
+default:
+  error (0, 0, "%s: Unknown DWARF DW_FORM_%d", dso->filename, *formp);
+  return FORM_ERROR;
+}
+
+  if (*formp == DW_FORM_block1)
+*ptrp += len;
+
+  return FORM_OK;
+}
+
+/* Part of read_dwarf2_line processing DWARF-4.  */
 static bool
-read_dwarf2_line (DSO *dso, uint32_t off, char *comp_dir)
+read_dwarf4_line (DSO *dso, unsigned char *ptr, char *comp_dir,
+		  struct line_table *table)
 {
-  unsigned char *ptr, *dir;
   unsigned char **dirt;
   uint32_t value, dirt_cnt;
   size_t comp_dir_len = !comp_dir ? 0 : strlen (comp_dir);
-  struct line_table *table;
-
-  if (get_line_table (dso, off, ) == false
-  || table == NULL)
-return false;
-
-  /* Skip to the directory table. The rest of the header has already
- been read and checked by get_line_table. */
-  ptr = debug_sections[DEBUG_LINE].data + off;
-  ptr += (4 /* unit len */
-	  + 2 /* version */
-	  + 4 /* header len */
-	  + 1 /* min instr len */
-	  + (table->version >= 4) /* max op per instr, if version >= 4 */
-	  + 1 /* default is stmt */
-	  + 1 /* line base */
-	  + 1 /* line range */
-	  + 1 /* 

[Rpm-maint] debugedit: Fix handling in caller for errors in called read_dwarf2_line.

[Mark Wielaard]

Hi,

I don't fully understand what this patch is intended to do. Why is it
necessary?

> commit 0da448c337d481f1c50af212246ceb213a7d80cc (HEAD -> master)
> Author: Jan Kratochvil 
> Date:   Mon Aug 17 21:46:47 2020 +0200
> 
> debugedit: Fix handling in caller for errors in called
> read_dwarf2_line.
> 
> diff --git a/tools/debugedit.c b/tools/debugedit.c
> index 8e85847d1..ff72759ca 100644
> --- a/tools/debugedit.c
> +++ b/tools/debugedit.c
> @@ -1155,7 +1155,7 @@ get_line_table (DSO *dso, size_t off, struct
> line_table **table)
>  if (lines->table[i].old_idx == off)
>{
> *table = >table[i];
> -   return false;
> +   return true;
>}

This breaks the contract of the function, which says:

   Gets a line_table at offset. Returns true if not yet know and
   successfully read, false otherwise.

The intention is to let the caller know whether the line_table has
already been handled or not earlier. It makes sure read_dwarf2_line
doesn't try parsing the same line table multiple times even if it is
referenced multiple times by different CUs.

Returning true in this case might make read_dwarf2_line parse and
replace the same line table multiple times.

>if (lines->size == lines->used)
> @@ -1621,7 +1621,8 @@ read_dwarf2_line (DSO *dso, uint32_t off, char 
> *comp_dir)
>  }
>  
>dso->lines.debug_lines_len += 4 + table->unit_length + table->size_diff;
> -  return table->replace_dirs || table->replace_files;
> +  need_stmt_update = table->replace_dirs || table->replace_files;
> +  return true;
>  }

Again this changes the documented contract of the function which says:

   Returns true if line_table needs to be rewrite either the dir or
   file paths.

I see you change that by setting need_stmt_update early, but it is
confusing to have the function do something different than documented.

>  /* Called during phase one, after the table has been sorted. */
> @@ -1939,9 +1940,11 @@ edit_attributes (DSO *dso, unsigned char *ptr, struct 
> abbrev_tag *t, int phase)
>   that).  Note that calculating the new size and offsets is done
>   separately (at the end of phase zero after all CUs have been
>   scanned in dwarf2_edit). */
> -  if (phase == 0 && found_list_offs
> -  && read_dwarf2_line (dso, list_offs, comp_dir))
> -need_stmt_update = true;
> +  if (found_list_offs && ! read_dwarf2_line (dso, list_offs, comp_dir))
> +{
> +  free (comp_dir);
> +  return NULL;
> +}

Why do you need to call read_dwarf2_line in all phases here?
The comment just before this code explains which work is done in which
phase, but with this change that seems no longer valid.
 
>free (comp_dir);
>  
> @@ -2059,7 +2062,10 @@ edit_info (DSO *dso, int phase, struct debug_section 
> *sec)
>  
>   ptr = edit_attributes (dso, ptr, t, phase);
>   if (ptr == NULL)
> -   break;
> +   {
> + htab_delete (abbrev);
> + return 1;
> +   }
> }
>  
>htab_delete (abbrev);

This seems to fix a minor memory leak. Looks OK.

Cheers,

Mark

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] [NFC] debugedit: Protect macro arguments by parentheses

[Mark Wielaard]

Hi,

The attached patch is part of the proposed patch series for PR/1332 and
looks good. It can be applied independently IMHO.

Cheers,

Mark
From aabd13f513eefe7e7db35a668e650a304def85d5 Mon Sep 17 00:00:00 2001
From: Jan Kratochvil 
Date: Mon, 17 Aug 2020 16:56:56 +0200
Subject: [PATCH] [NFC] debugedit: Protect macro arguments by parentheses

---
 tools/debugedit.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/debugedit.c b/tools/debugedit.c
index c2884933c..8e85847d1 100644
--- a/tools/debugedit.c
+++ b/tools/debugedit.c
@@ -233,7 +233,7 @@ typedef struct
   int shift = 0;			\
   do	\
 {	\
-  c = *ptr++;			\
+  c = *(ptr)++;			\
   ret |= (c & 0x7f) << shift;	\
   shift += 7;			\
 } while (c & 0x80);			\
@@ -251,7 +251,7 @@ typedef struct
   valv >>= 7;			\
   if (valv)\
 	c |= 0x80;			\
-  *ptr++ = c;			\
+  *(ptr)++ = c;			\
 }	\
   while (valv);\
 })
@@ -311,7 +311,7 @@ strptr (DSO *dso, int sec, off_t offset)
 }
 
 
-#define read_8(ptr) *ptr++
+#define read_8(ptr) *(ptr)++
 
 #define read_16(ptr) ({	\
   uint16_t ret = do_read_16 (ptr);			\
@@ -328,13 +328,13 @@ strptr (DSO *dso, int sec, off_t offset)
 REL *relptr, *relend;
 int reltype;
 
-#define do_read_32_relocated(ptr) ({			\
-  uint32_t dret = do_read_32 (ptr);			\
+#define do_read_32_relocated(xptr) ({			\
+  uint32_t dret = do_read_32 (xptr);			\
   if (relptr)		\
 {			\
-  while (relptr < relend && relptr->ptr < ptr)	\
+  while (relptr < relend && relptr->ptr < (xptr))	\
 	++relptr;	\
-  if (relptr < relend && relptr->ptr == ptr)	\
+  if (relptr < relend && relptr->ptr == (xptr))	\
 	{		\
 	  if (reltype == SHT_REL)			\
 	dret += relptr->addend;			\
-- 
2.28.0

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] debugedit: Distinguish files from directories in src list file (#1436)

[Mark Wielaard]

Hi,

On Sun, 2020-11-15 at 07:20 -0800, Vitalio wrote:
> Append / to directories in source file list (for `-l
> option) to allow
> quickly distinguish them from regular files (to avoid adding them raw
> into %files section). This is needed for ALT for our debuginfo
> processing to speed things up.

I understand you would like to make sure all comp_dirs end with a
trailing '/' character. But I don't think unconditionally adding a '/'
is the right thing to do:

> diff --git a/tools/debugedit.c b/tools/debugedit.c
> index c2884933c..2eb912c72 100644
> --- a/tools/debugedit.c
> +++ b/tools/debugedit.c
> @@ -1923,7 +1923,7 @@ edit_attributes (DSO *dso, unsigned char *ptr, struct 
> abbrev_tag *t, int phase)
>const char *p = skip_dir_prefix (comp_dir, base_dir);
>if (p != NULL && p[0] != '\0')
>  {
> -   size_t size = strlen (p) + 1;
> +   size_t size = strlen (p);
> while (size > 0)
>   {
> ssize_t ret = write (list_file_fd, p, size);
> @@ -1932,6 +1932,10 @@ edit_attributes (DSO *dso, unsigned char *ptr, struct 
> abbrev_tag *t, int phase)
> size -= ret;
> p += ret;
>   }
> +   /* Output trailing dir separator to distinguish them quickly from
> +  regular files. */
> +   if (size == 0)
> + write (list_file_fd, "/", 2);
>   }
>  }

Note for others, the list_file contains zero terminated file names.
That is why we want to write size = strlen (p) + 1 characters, so that
it includes the '\0' terminator.

After the while loop size is always zero, unless an error occurred and
not all characters were written. So this code would add a "/"
(including a termination '\0') always, even if there was already one.
So I think you actually want to test that p[0] != '/' before adding the
string "/", but just write (list_file_fd, "", 1) otherwise.

BTW. How does this help you speed things up?

Cheers,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] debugedit: Do not 'edit_dwarf2' when just extracting build-id (#1435)

[Mark Wielaard]

On Sun, Nov 15, 2020 at 07:11:02AM -0800, Vitalio wrote:
> No need to call edit_dwarf2() if debugedit is invoked just to extract
> build-id (with `-i -n).

Yes, but you also have to check base_dir && dest_dir aren't set.

> Otherwise, we will get `DWARF version 0
> unhandled warning for compressed .debug files:
> 
>   /usr/lib/rpm/debugedit: ./usr/lib/debug/...e.ko.debug: DWARF version 0 
> unhandled
> 
> Context: We have kernel modules elfcompressed in ALT.

In general debugedit doesn't handle compressed ELF sections at the
moment.  You could do as Fedora does and run eu-elfcompress -t none on
the file first to make sure it doesn't contain any compressed ELF
sections.

Cheers,

Mark

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] Remove set, but unused, variables in macro.c and rpmlua.c

[Mark Wielaard]

macro.c: In function ‘mbopt’:
macro.c:895:19: warning: unused variable ‘me’ [-Wunused-variable]
  895 | rpmMacroEntry me = mb->me;
  |   ^~

rpmlua.c: In function ‘fd_seek’:
rpmlua.c:985:22: warning: unused variable ‘mode’ [-Wunused-variable]
  985 | static const int mode[] = {SEEK_SET, SEEK_CUR, SEEK_END};
  |  ^~~~
---
 rpmio/macro.c  | 1 -
 rpmio/rpmlua.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/rpmio/macro.c b/rpmio/macro.c
index 800861418..1edcb39e6 100644
--- a/rpmio/macro.c
+++ b/rpmio/macro.c
@@ -892,7 +892,6 @@ static void splitQuoted(ARGV_t *av, const char * str, const 
char * seps)
 static int mbopt(int c, const char *oarg, int oint, void *data)
 {
 MacroBuf mb = data;
-rpmMacroEntry me = mb->me;
 char *name = NULL, *body = NULL;
 
 /* Define option macros. */
diff --git a/rpmio/rpmlua.c b/rpmio/rpmlua.c
index 8314a296c..2ea985e6e 100644
--- a/rpmio/rpmlua.c
+++ b/rpmio/rpmlua.c
@@ -982,7 +982,6 @@ static int fd_flush(lua_State *L)
 
 static int fd_seek(lua_State *L)
 {
-static const int mode[] = {SEEK_SET, SEEK_CUR, SEEK_END};
 static const char *const modenames[] = {"set", "cur", "end", NULL};
 FD_t *fdp = checkfd(L, 1);
 int op = luaL_checkoption(L, 2, "cur", modenames);
-- 
2.18.4

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] elfdeps: Generate dependencies on non-executable shared libraries (#1393)

[Mark Wielaard]

Hi,

On Mon, 2020-10-12 at 00:35 -0700, Panu Matilainen wrote:
> The problem here is that we're trying to classify a hermaphrodite
> entity into one of two sexes. ET_EXEC is clear, but ET_DYN can be
> either an executable, a dynamic shared library *or both*. Until we
> embrace this fact, we'll get it wrong half of the time. 
> 
> So we need to stop trying to decide what *it is* and instead ask it
> what it's *capable of*, and act accordingly.
> libmagic doesn't know nearly enough to handle this, so the logic
> needs to live inside elfdeps or we need to enhance the classifier
> somehow (wouldn't be a bad thing at all but perhaps out of scope
> here). Which means a split on the .attr level likely isn't going to
> be meaningful.
> 
> If we want to preserve the behavior where executables can disable
> dependency generation by stripping x, we need to come up with a new
> way of defining an executable. Given the nature of modern ELF, I
> think the only meaningful way is to draw the line at: regardless of
> it's possible other capabilities, does it do something if executed?
> Which AIUI in technical terms comes down to: does it have PT_INTERP?
> elfdeps looks that info up as it is, but it's not wired in a
> meaningful way at the moment.

That is indeed a tricky question to answer. Note that elfutils since
0.177 comes with a tool eu-elfclassify to answer precisely these
questions:
https://gnu.wildebeest.org/blog/mjw/2019/08/15/elfutils-0-177-released-with-eu-elfclassify/

You might want to use that or study the code to see how it determines
some of the corner cases. Specifically you may want to lookup
is_executable () and is_program (), is_library () and is_shared () for
some of the tricky corner cases:

  --executable   File is (primarily) an ELF program executable (not
 primarily a DSO)
  --program  File is an ELF program executable (might also be a
 DSO)

  --library  File is an ELF shared object (DSO) (might also be
 an executable)
  --shared   File is (primarily) an ELF shared object (DSO)
 (not primarily an executable)

Cheers,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] debugedit: Implement DWARF-5. (#1332)

[Mark Wielaard]

Hi Jan,

On Sun, 2020-08-16 at 15:02 -0700, Jan Kratochvil wrote:
> -- Commit Summary --
> 
>   * debugedit: Implement DWARF-5.
> 
> -- File Changes --
> 
> M tests/debugedit.at (249)
> M tools/debugedit.c (914)
> 
> -- Patch Links --
> 
> https://github.com/rpm-software-management/rpm/pull/1332.patch
> https://github.com/rpm-software-management/rpm/pull/1332.diff

Although I can chase down the links to github, I personally find simple
patches to the list easier to review than having to chase links to
github.

The commit summary is "Implement DWARF-5", but it mixes that plus
support for an alternative compiler and debug-types in one commit.
Would it be possible to split this in 3 patches, one for basic DWARF-5
support, one for testing against an alternative compiler (which I
assume also includes an alternative .debug_line representation from
what gcc produces) and one for debug-types in DWARF5? That would make
reviewing this simpler.

Thanks,

Mark
> 
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] debugedit: Fix missing relocation of .debug_types section. (#1323)

[Mark Wielaard]

On Sat, Aug 08, 2020 at 07:04:03AM +0200, Jan Kratochvil wrote:
> On Fri, 07 Aug 2020 13:55:35 +0200, Mark Wielaard wrote:
> > But I pulled
> > that branch and reviewed the actual commits (1d080e02 and c804a960).
> 
> git clone -b types g...@github.com:jankratochvil/rpm.git rpm-types
> 
> commit 8b5bbcc6d586be50b6a251256c39c3b0332b1f2b
> debugedit: Fix missing relocation of .debug_types section.
> commit 1d080e02409d181169d3aec2a19192418f253fd3
> [NFC] debugedit: Move code from edit_dwarf2() to edit_info().
> 
> 
> > It would be better to have a separate RPM_DEBUGEDIT_TYPES_SETUP that
> > does that and leave RPM_DEBUGEDIT_SETUP as is. Otherwise we change the
> > existing tests too.
> 
> done as: RPM_DEBUGEDIT_SETUP([-fdebug-types-section])

Nice. Using $1 is better than what I suggested.

Commits look good. Should go into the main branch IMHO.

Thanks,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] debugedit: Fix missing relocation of .debug_types section. (#1323)

[Mark Wielaard]

Hi Jan,

On Sat, 2020-08-01 at 11:23 +0200, Jan Kratochvil wrote:
> debugedit: Fix missing relocation of .debug_types section.
> https://github.com/rpm-software-management/rpm/pull/1323

I believe our mail review comments don't make it to that website. And
given that there are some forced updates to that pull branch. It is
sometimes hard to see which version we are discussing. But I pulled
that branch and reviewed the actual commits (1d080e02 and c804a960).

> > But here is a review inline:
> > If it is for the same lines that are moved from edit_dwarf2 () to
> > edit_info () below then it is fine, but if you do it in a separate
> > commit maybe also factor out edit_info () already? To make the next
> > commit easier to read.
> 
> Yes, that was my original intention but I made a mistake, fixed now.

commit 1d080e02 (after checking with diff -u -w) looks reasonable.
I would check that into the main branch as is.

> > This will only work for executables or shared librareis, not for
> > (ET_REL) object files (or kernel modules) because those come with more
> > than 1 (comdat) .debug_types section. This is probably fine. But if you
> > expect that .debug_types will also appear in relocatable files, then
> > you might want to look at what edit_dwarf2() does for .debug_macro,
> > which might also appear multiple times.
> 
> done

commit c804a960
Looks good. Thanks.

> > You do include a testcase for the relocatable object case:
> 
> done
> 
> 
> > I would suggest extending the testcase a little to have multiple
> > larger
> > structs, plus some small field names. e.g. add another struct in
> > foobar.h:
> 
> done

The test cases loook pretty comprehensive now. Nice.
One request though. In commit c804a960 you adjust RPM_DEBUGEDIT_SETUP
as follows:

> diff --git a/tests/debugedit.at b/tests/debugedit.at
> index cae3f4384..aa878d6fb 100644
> --- a/tests/debugedit.at
> +++ b/tests/debugedit.at
> @@ -36,11 +36,11 @@ cp "${abs_srcdir}"/data/SOURCES/foobar.h subdir_headers
>  cp "${abs_srcdir}"/data/SOURCES/baz.c .
>  
>  # First three object files (foo.o subdir_bar/bar.o and baz.o)
> -gcc -g3 -Isubdir_headers -c subdir_foo/foo.c
> +gcc -g3 -Isubdir_headers -fdebug-types-section -c subdir_foo/foo.c
>  cd subdir_bar
> -gcc -g3 -I../subdir_headers -c bar.c
> +gcc -g3 -I../subdir_headers -fdebug-types-section -c bar.c
>  cd ..
> -gcc -g3 -I$(pwd)/subdir_headers -c $(pwd)/baz.c
> +gcc -g3 -I$(pwd)/subdir_headers -fdebug-types-section -c $(pwd)/baz.c
>  
>  # Then a partially linked object file (somewhat like a kernel module).
>  # This will still have relocations between the debug sections.

It would be better to have a separate RPM_DEBUGEDIT_TYPES_SETUP that
does that and leave RPM_DEBUGEDIT_SETUP as is. Otherwise we change the
existing tests too. And I believe we want to make sure we test those
with the default debug flags.

> > Maybe we can just remove that warning.
> 
> done

Looks good. Thanks.

> > This looks OK. But just before this the sections that have been changed
> > are marked "dirty", you probably want to mark DEBUG_TYPES also dirty if
> > it has been updated in any way (otherwise it isn't guaranteed the data
> > is written to disk, although it often will be).
> 
> done

Nice catch on the dirty_section () having to follow secp->next (that
was a latent bug for the DEBUG_MACRO case).

> > To make sure you test the case where there are multiple debug line
> > table offsets in your .debug_type sections, you might want to add
> > something like the following to bar.c:
> 
> done

Nice work on the tests.

I read the whole commit c804a960 and it looks good.

The only change I would like, as explained above, is to have a separate
RPM_DEBUGEDIT_TYPES_SETUP. With that it should be good to commit to the
main branch.

Thanks,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] debugedit: Fix missing relocation of .debug_types section. (#1323)

[Mark Wielaard]

Hi Florian,

On Fri, 2020-07-31 at 14:24 +0200, Florian Festi wrote:
> Mark can you have a look at this, please?

Sure. But somehow I never got this email through the mailinglist, so I
don't know how to reply so that my comments show up on that pull. Also,
it seems there are various force pushes that mess up the commit
ids/patches. But here is a review inline:

> > * [NFC] debugedit: Reindent edit_dwarf2().

This is basically:

diff --git a/tools/debugedit.c b/tools/debugedit.c
index 9f8dcd0fb..dcacb23b7 100644
--- a/tools/debugedit.c
+++ b/tools/debugedit.c
@@ -2100,8 +2100,9 @@ edit_dwarf2 (DSO *dso)
   return 1;
 }
 
-  if (debug_sections[DEBUG_INFO].data != NULL)
-{
+  if (debug_sections[DEBUG_INFO].data == NULL)
+return 0;
+
   unsigned char *ptr, *endcu, *endsec;
   uint32_t value;
   htab_t abbrev;
@@ -2480,7 +2481,6 @@ edit_dwarf2 (DSO *dso)
  macro_sec = macro_sec->next;
}
 }
-}
 
   return 0;
 }

Which is logically correct, but I don't know why you would do that.
It unnecessarily changes ~400 source lines, causing it to make it
slightly harder to track when which change was made by whom for what.
If it is for the same lines that are moved from edit_dwarf2 () to
edit_info () below then it is fine, but if you do it in a separate
commit maybe also factor out edit_info () already? To make the next
commit easier to read.

> >   * debugedit: Fix missing relocation of .debug_types section.

This will only work for executables or shared librareis, not for
(ET_REL) object files (or kernel modules) because those come with more
than 1 (comdat) .debug_types section. This is probably fine. But if you
expect that .debug_types will also appear in relocatable files, then
you might want to look at what edit_dwarf2() does for .debug_macro,
which might also appear multiple times.

You do include a testcase for the relocatable object case:

+# ===
+# Make sure -fdebug-types-section has updated strings in partial linked object.
+# ===
+AT_SETUP([debugedit .debug_types partial])
+AT_KEYWORDS([debugtypes] [debugedit])
+RPM_DEBUGEDIT_SETUP
+
+AT_DATA([expout],
+[fieldname1
+])
+
+AT_CHECK([[debugedit -b $(pwd) -d /foo/bar/baz ./foobarbaz.part.o]])
+AT_CHECK([[
+readelf --debug-dump=info ./foobarbaz.part.o \
+   | sed -n '/Abbrev Number:.*DW_TAG_type_unit/,/^$/p' \
+| sed -n 's/^.*> *DW_AT_name *:.* \(fieldname.\)$/\1/p'
+]],[0],[expout])
+
+AT_CLEANUP

Which is great, thanks for adding that (and the single object files and
exectuable testcases). The tests look good, but won't catch the
multiple .debug_types sections case (and only test indirect strings in
.debug_str, not the small strings embedded in .debug_info).

I would suggest extending the testcase a little to have multiple larger
structs, plus some small field names. e.g. add another struct in
foobar.h:

struct types_section
{
  char t;
  int n1;
};

which then gets referenced in both foo.c and bar.c as

struct types_section debug_var_foo;

and

struct types_section debug_var_bar;

That way you get at least two .debug_type (comdat) sections and one
with "static" strings.

> >  * *M* tools/debugedit.c

> diff --git a/tools/debugedit.c b/tools/debugedit.c
> index dcacb23b7..f635c0c62 100644
> --- a/tools/debugedit.c
> +++ b/tools/debugedit.c
> @@ -1459,7 +1459,7 @@ edit_dwarf2_line (DSO *dso)
> adjustments needed in the debug_list data structures. Returns true
> if line_table needs to be rewrite either the dir or file paths. */
>  static bool
> -read_dwarf2_line (DSO *dso, uint32_t off, char *comp_dir)
> +read_dwarf2_line (DSO *dso, int info_scn_ix, uint32_t off, char *comp_dir)
>  {
>unsigned char *ptr, *dir;
>unsigned char **dirt;
> @@ -1470,7 +1470,7 @@ read_dwarf2_line (DSO *dso, uint32_t off, char 
> *comp_dir)
>if (get_line_table (dso, off, ) == false
>|| table == NULL)
>  {
> -  if (table != NULL)
> +  if (table != NULL && info_scn_ix == DEBUG_INFO)
> error (0, 0, ".debug_line offset 0x%x referenced multiple times",
>off);
>return false;

Maybe we can just remove that warning. It isn't really "illegal" to
reference a debug line table multiple times. All we really care about
is that we only process it once (and only process those that are
actually used).

> @@ -1644,7 +1644,8 @@ find_new_list_offs (struct debug_lines *lines, size_t 
> idx)
> abbrev data is consumed. In phase zero data is collected, in phase one
> data might be replaced/updated.  */
>  static unsigned char *
> -edit_attributes (DSO *dso, unsigned char *ptr, struct abbrev_tag *t, int 
> phase)
> +edit_attributes (DSO *dso, int info_scn_ix, unsigned char *ptr,
> +struct abbrev_tag *t, int phase)
>  {
>int i;
>uint32_t list_offs;
> @@ -1942,7 +1943,7 @@ edit_attributes (DSO *dso, unsigned char *ptr, struct 
> abbrev_tag *t, int phase)
>   separately (at the end of phase zero after all CUs have been
>   scanned in dwarf2_edit). */
>

Re: [Rpm-maint] [rpm-software-management/rpm] 4.15.1: sepdebugcrcfix is not able to locate source files (#1207)

[Mark Wielaard]

Hi Tomasz,

On Sat, 2020-07-25 at 06:44 -0700, Tomasz Kłoczko wrote:
> Any update or progress?

Unfortunately it seems to be a GCC bug:
".debug_line with LTO refers to bogus file-names"
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93865
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Please support smaller build-ids (#950)

[Mark Wielaard]

Hi Frank,

Thanks so much for trying to come up with actual numbers!

On Tue, 2019-11-26 at 13:07 -0800, Frank Ch. Eigler wrote:
> Argh, very sorry @espindola but I was 3 orders of magnitude too high
> with my extrapolated numbers.  I hereby hand back my math card.  The
> collision probability is more in the 10e-6 range.

This seems mood since the issue is already closed and lld will probably
switch to a 128 or 160 bit build-id anyway. But I am still interested
in the actual numbers and I am now also a little confused.

When you say you were off by 3 orders of magnitude to you mean that for
one architecture (x86_64) and one release (fedora 30) you estimate that
there are ~600.000 build-ids (instead of ~600.000.000) because that is
the number of executable artifacts? Or that you extrapolated it (which
number?) wrongly when looking at the number of arches (i386 and x86_64
can be installed concurrently) and actual distros (3 normally) that
overlap in maintenance?

Or asked differently, which number of build-ids are you expecting to
come to a collision probability of 10e-6 when the build-id is just
64bits?

Thanks,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Please support smaller build-ids (#950)

[Mark Wielaard]

On Sat, 2019-11-23 at 09:53 -0800, Rafael Ávila de Espíndola wrote:
> 
> If we really need 128 bits, which hash would you suggest for a fast
> buildid? Farmhash comes to mind.

128 bits is the minimum that GNU binutils ld and gold support, you can
choose between uuid (128bits), md5 (128bits) and sha1 (160bits). Where
sha1 is the default. If you want maximum compatibility you would use a
hash that is 160bits/20bytes. I have seen code that simply hardcodes
build-ids being 20 bytes (those should be fixed of course, but that
might take time).

But I don't know if 64bits/8bytes is enough, assuming the hash code is
good the the birthday problem probabilty table shows the difference
between using 64bit and 128bit hashes:
https://en.wikipedia.org/wiki/Birthday_problem#Probability_table

I'll ask some people who have tried to index all executables of a
distribution what number of executables they have encountered.
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Remove unused variable id in find-debuginfo.sh (#825)

[Mark Wielaard]

Hi,

On Mon, Aug 26, 2019 at 02:44:22AM -0700, Tom de Vries wrote:
> There's code setting variable id in scripts/find-debuginfo.sh, but there's no
> subsequent use.  The only use of $id in the script is in do_file, where it is
> a local variable.
> 
> The variable setting was introduced in commit 6b3b435fa "Add dwz debuginfo
> compression support" in combination with a subsequent make_id_link using the
> variable, but the make_id_link was removed in commit bbfe1f86b
> "Add build-id links to rpm for all ELF files".
> 
> Remove the unused variable.

Thanks for looking through the history.
This makes sense, the id variable should have been removed when
the only usage of it for make_id_link was removed. Now it just does
extra work for nothing.

Cheers,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Add --zlib-compress-debug-sections to find-debug-info.sh (#795)

[Mark Wielaard]

Hi,

Sorry for the late reply.

On Thu, 2019-07-18 at 05:40 -0700, Tom de Vries wrote:
> The script find-debuginfo.sh has a --run-dwz switch to compress the debug
> info using dwz.
> 
> Add a similar switch --zlib-compress-debug-sections that compresses the debug
> info using objcopy --compress-debug-sections.

Note that eu-elfcompress does the same. Not that it really should
matter which tool does it. Currently find-debuginfo.sh uses tools from
both binutils and elfutils, it might be nice to only depend on one set
of tools (my preference would obviously be to migrate everything to the
elfutils toolset).

There is also zlib-gnu, the older, pre-standard, compression. Dunno if
we want to support that too.

> The two modes of compression can be used independently, so we can use one, or
> the other, or both.

I do think that this is a nice option to have in theory. But I haven't
tried it myself yet

> https://github.com/rpm-software-management/rpm/pull/795.patch
> https://github.com/rpm-software-management/rpm/pull/795.diff

-  id="`readelf -Wn 
"${RPM_BUILD_ROOT}/usr/lib/debug/.dwz/${dwz_multifile_name}" \
-2>/dev/null | sed -n 's/^Build ID: \([0-9a-f]\+\)/\1/p'`"

I am slightly surprised we don't seem to use the dwz build-id. Could
you double check we don't actually need that id?

Also, is there really no distro that is contemplating to use this?
If not, then maybe it is better to not add the extra complexity till
someone really wants it.

Cheers,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Compress annobin notes (#751)

[Mark Wielaard]

Hi Nick,

On Fri, 2019-06-14 at 09:29 -0700, nickclifton wrote:
> This is a request to add support for compressing annobin notes found
> in executable binaries built on Fedora and RHEL systems.
> 
> The annobin project adds a note section to binary files describing
> the security hardening features of how they were
> built.  Unfortunately these notes can get quite large, especially for
> projects that use lots of object files.  The objcopy program from the
> binutils package has an option to reduce the size of these notes by
> eliminating empties and merging duplicates.  If the binary does not
> contain any annobin notes then the objcopy will take no noticeable
> amount of time.  In fact even if the file does contain annobin notes
> the merging process is relatively fast and it is unlikely to add any
> significant amount of time to the overall build process.
> [...]
> diff --git a/scripts/find-debuginfo.sh b/scripts/find-debuginfo.sh
> index d75da1108..14d9ed901 100755
> --- a/scripts/find-debuginfo.sh
> +++ b/scripts/find-debuginfo.sh
> @@ -296,6 +296,8 @@ add_minidebug()
>xz "$mini_debuginfo"
>mini_debuginfo="${mini_debuginfo}.xz"
>objcopy --add-section .gnu_debugdata="$mini_debuginfo" "$binary"
> +  # Compress any annobin notes in the original binary.
> +  objcopy --merge-notes "$binary"
>rm -f "$dynsyms" "$funcsyms" "$keep_symbols" "$mini_debuginfo"
>  }
>  
> 

I don't think this should be part of add_minidebug (). It is something
that would need to happen even if we don't run add_minidebug (). Also
add_minidebug () runs after stripping/splitting the main ELF file into
a .debug file (which also gets a copy of all notes).

I noticed older binutils objcopy don't seem to know about --merge-
notes. And produce large errors when trying to run.

So I think this should be something like this, in do_file (), before
the binary is actually stripped.

diff --git a/scripts/find-debuginfo.sh b/scripts/find-debuginfo.sh
index d75da11..b6a343e 100755
--- a/scripts/find-debuginfo.sh
+++ b/scripts/find-debuginfo.sh
@@ -405,6 +405,10 @@ do_file()
 fi
   fi
 
+  # Compress any annobin notes in the original binary.
+  # Ignore any errors, since older objcopy don't support --merge-notes
+  objcopy --merge-notes "$f" 2>/dev/null || true
+
   # A binary already copied into /usr/lib/debug doesn't get stripped,
   # just has its file names collected and adjusted.
   case "$dn" in

Cheers,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] debugedit: Add some testcases and handle .debug_macro (gcc -g3)

[Mark Wielaard]

Hi,

debugedit wouldn't properly handle gcc -g3 which generates .debug_macro
sections. The sections would contain references to .debug_str which
debugedit rewrites. But it would not update the references in .debug_macro
which would upset various tools.  For example gdb:
https://bugzilla.redhat.com/show_bug.cgi?id=1535174

Michael Schroeder had a patch for that, but that didn't handle relocations
(which happen in e.g. linux kernel modules). So I added support for that.
(patch four).

debugedit didn't have any specific tests. It was only tested as part of
the whole rpmbuild debuginfo pipeline. So I first added a bunch of test
cases (the first two patches). That helped make sure my debugedit relocation
code refactoring was working as expected (the third patches).

It also found an small issue (under valgrind) with the .debug_line
rewriting code (the last patch).

[PATCH 1/5] Add some color to the tests if available and the terminal
[PATCH 2/5] Add some debugedit tests.
[PATCH 3/5] debugedit: Refactor reading/writing of relocated values.
[PATCH 4/5] Handle .debug_macro in debugedit.
[PATCH 5/5] debugedit: Make sure .debug_line old/new idx start equal.

Cheers,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 4/5] Handle .debug_macro in debugedit.

[Mark Wielaard]

When compiling with -g3 gcc will generate a .debug_macro section
which has pointers to the .debug_str section. Since we might rewrite
the .debug_str section, we also need to update any .debug_macro
pointers.

Updated the debugedit.at testcase by building everything with -g
and add various checks to see the .debug_macro section looks OK
after running debugedit. Added a new rpmbuild.at testcase to check
handing of .debug_macro in the whole rpmbuild debuginfo pipeline
to double check the separate .debug file also contains the macros.

Original patch by Michael Schroeder . Extended by
Mark Wielaard  to deal with relocations and possible
multiple COMDAT .debug_macro sections.
---
 tests/Makefile.am  |   1 +
 tests/data/SPECS/hello-g3.spec |  60 +
 tests/debugedit.at |  79 -
 tests/rpmbuild.at  |  33 +++
 tools/debugedit.c  | 196 +++--
 5 files changed, 356 insertions(+), 13 deletions(-)
 create mode 100644 tests/data/SPECS/hello-g3.spec

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 6f909d7..046c912 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -49,6 +49,7 @@ EXTRA_DIST += data/SPECS/hello2.spec
 EXTRA_DIST += data/SPECS/hello2cp.spec
 EXTRA_DIST += data/SPECS/hello2ln.spec
 EXTRA_DIST += data/SPECS/hello2-suid.spec
+EXTRA_DIST += data/SPECS/hello-g3.spec
 EXTRA_DIST += data/SPECS/foo.spec
 EXTRA_DIST += data/SPECS/globtest.spec
 EXTRA_DIST += data/SPECS/versiontest.spec
diff --git a/tests/data/SPECS/hello-g3.spec b/tests/data/SPECS/hello-g3.spec
new file mode 100644
index 000..55bbb4a
--- /dev/null
+++ b/tests/data/SPECS/hello-g3.spec
@@ -0,0 +1,60 @@
+Summary: hello-g3 -- double hello, world rpm, .debug_macro -g3
+Name: hello-g3
+Version: 1.0
+Release: 1
+Group: Utilities
+License: GPL
+Distribution: RPM test suite.
+Vendor: Red Hat Software
+Packager: Red Hat Software 
+URL: http://www.redhat.com
+Source0: hello-1.0.tar.gz
+Patch0: hello-1.0-modernize.patch
+Prefix: /usr
+
+%description
+Simple rpm demonstration.
+
+%prep
+%setup -q -n hello-1.0
+%patch0 -p1 -b .modernize
+
+%build
+make CFLAGS="-g3 -O1 -DDEBUG_DEFINE=1"
+mv hello hello-g3
+make CFLAGS="-g3 -O2 -D_FORTIFY_SOURCE=2"
+
+%install
+rm -rf $RPM_BUILD_ROOT
+mkdir -p $RPM_BUILD_ROOT/usr/local/bin
+make DESTDIR=$RPM_BUILD_ROOT install
+cp hello-g3 $RPM_BUILD_ROOT/usr/local/bin/
+
+%clean
+rm -rf $RPM_BUILD_ROOT
+
+%pre
+
+%post
+
+%preun
+
+%postun
+
+%files
+%defattr(-,root,root)
+%doc   FAQ
+#%readme README
+#%license COPYING
+%attr(0751,root,root)  /usr/local/bin/hello
+%attr(0751,root,root)  /usr/local/bin/hello-g3
+
+%changelog
+* Mon Jun  3 2019 Mark Wielaard 
+- Create hello-g3 for -g3 .debug_macro testing.
+
+* Wed May 18 2016 Mark Wielaard 
+- Add hello2 for dwz testing support.
+
+* Tue Oct 20 1998 Jeff Johnson 
+- create.
diff --git a/tests/debugedit.at b/tests/debugedit.at
index 8b724d3..7e26085 100644
--- a/tests/debugedit.at
+++ b/tests/debugedit.at
@@ -35,11 +35,11 @@ cp "${abs_srcdir}"/data/SOURCES/foobar.h subdir_headers
 cp "${abs_srcdir}"/data/SOURCES/baz.c .
 
 # First three object files (foo.o subdir_bar/bar.o and baz.o)
-gcc -g -Isubdir_headers -c subdir_foo/foo.c
+gcc -g3 -Isubdir_headers -c subdir_foo/foo.c
 cd subdir_bar
-gcc -g -I../subdir_headers -c bar.c
+gcc -g3 -I../subdir_headers -c bar.c
 cd ..
-gcc -g -I$(pwd)/subdir_headers -c $(pwd)/baz.c
+gcc -g3 -I$(pwd)/subdir_headers -c $(pwd)/baz.c
 
 # Then a partially linked object file (somewhat like a kernel module).
 # This will still have relocations between the debug sections.
@@ -47,7 +47,7 @@ ld -r -o foobarbaz.part.o foo.o subdir_bar/bar.o baz.o
 
 # Create an executable. Relocations between debug sections will
 # have been resolved.
-gcc -g -o foobarbaz.exe foo.o subdir_bar/bar.o baz.o
+gcc -g3 -o foobarbaz.exe foo.o subdir_bar/bar.o baz.o
 ]])
 
 # ===
@@ -320,3 +320,74 @@ readelf --debug-dump=line ./foobarbaz.exe \
 ]],[0],[expout])
 
 AT_CLEANUP
+
+# ===
+# Make sure .debug_macro strings are still there
+# in objects.
+# ===
+AT_SETUP([debugedit .debug_macro objects])
+AT_KEYWORDS([debuginfo] [debugedit])
+RPM_DEBUGEDIT_SETUP
+
+# We expect 3 for each compile unit.
+AT_DATA([expout],
+[NUMBER 42
+NUMBER 42
+NUMBER 42
+])
+
+AT_CHECK([[debugedit -b $(pwd) -d /foo/bar/baz ./foo.o]])
+AT_CHECK([[debugedit -b $(pwd) -d /foo/bar/baz ./subdir_bar/bar.o]])
+AT_CHECK([[debugedit -b $(pwd) -d /foo/bar/baz ./baz.o]])
+AT_CHECK([[
+readelf --debug-dump=macro foo.o subdir_bar/bar.o baz.o \
+| grep NUMBER | rev | cut -d: -f1 | rev | cut -c2-
+]],[0],[expout])
+
+AT_CLEANUP
+
+# ===
+# Make sure .debug_macro strings are still there
+# in partial linked object.
+# ===
+AT_SETUP([debugedit .debug_macro partial])
+AT_KEYWORDS([debuginfo] [debugedit])
+RPM_DEBUGEDIT_SETUP
+
+# We expect 3 for each compile unit.
+AT_DATA([expout],
+[NUMBER 42
+NUMBER 42
+NUMBER 42
+])
+
+AT_CHE

[Rpm-maint] [PATCH 1/5] Add some color to the tests if available and the terminal supports it.

[Mark Wielaard]

While working on some new testcases it helped to see the test results
in color. We could all use some extra color while working on dull
tasks.
---
 tests/local.at | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/local.at b/tests/local.at
index 42eef1c..42c799b 100644
--- a/tests/local.at
+++ b/tests/local.at
@@ -35,4 +35,7 @@ RPMPY_CHECK([$2], [$3], [$4])
 AT_CLEANUP
 ])
 
+# Enable colored test output if available
+m4_ifdef([AT_COLOR_TESTS], [AT_COLOR_TESTS])
+
 AT_INIT
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 5/5] debugedit: Make sure .debug_line old/new idx start equal.

[Mark Wielaard]

Found by running the debugedit tests under valgrind.
If the old and new .debug_line offset isn't changed then we might
write out an uninitialized new_idx.
---
 tools/debugedit.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/debugedit.c b/tools/debugedit.c
index 84483ef..9f8dcd0 100644
--- a/tools/debugedit.c
+++ b/tools/debugedit.c
@@ -1177,6 +1177,7 @@ get_line_table (DSO *dso, size_t off, struct line_table 
**table)
   *table = NULL;
 
   t->old_idx = off;
+  t->new_idx = off;
   t->size_diff = 0;
   t->replace_dirs = false;
   t->replace_files = false;
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 2/5] Add some debugedit tests.

[Mark Wielaard]

debugedit didn't have any direct tests, it was only tested indirectly
through some other debuginfo testcases.  So add some testcases that
just test debugedit functionality directly.

The tests create different kinds of ELF files (object files, partially
linked files and executables) and run debugedit path replacements on
them, checking that the resulting .debug_str, .debug_info and .debug_line
sections look sane.

Testcases verified against various different gcc and binutils versions.
gcc 4.8 and gcc 6 generate slightly different .debuginfo

Also add debugedit --version. Which makes it easy to see we are
testing the correct version/install of debugedit in rpmtests.log.

Adjust autotest paths to include rpmlibexecdir, where debuginfo
is installed.  Note that rpmlibexecdir != libexecdir (see rpm.am).
---
 tests/Makefile.am   |   8 +-
 tests/atlocal.in|   2 +-
 tests/data/SOURCES/bar.c|   9 ++
 tests/data/SOURCES/baz.c|  10 ++
 tests/data/SOURCES/foo.c|   7 +
 tests/data/SOURCES/foobar.h |   6 +
 tests/debugedit.at  | 322 
 tests/rpmtests.at   |   1 +
 tools/debugedit.c   |  10 ++
 9 files changed, 373 insertions(+), 2 deletions(-)
 create mode 100644 tests/data/SOURCES/bar.c
 create mode 100644 tests/data/SOURCES/baz.c
 create mode 100644 tests/data/SOURCES/foo.c
 create mode 100644 tests/data/SOURCES/foobar.h
 create mode 100644 tests/debugedit.at

diff --git a/tests/Makefile.am b/tests/Makefile.am
index abad3f4..6f909d7 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -18,6 +18,7 @@ TESTSUITE_AT += rpmverify.at
 TESTSUITE_AT += rpmdb.at
 TESTSUITE_AT += rpmbuild.at
 TESTSUITE_AT += rpmbuildid.at
+TESTSUITE_AT += debugedit.at
 TESTSUITE_AT += rpmi.at
 TESTSUITE_AT += rpmvercmp.at
 TESTSUITE_AT += rpmdeps.at
@@ -99,6 +100,10 @@ EXTRA_DIST += data/keys/rpm.org-rsa-2048-test.pub
 EXTRA_DIST += data/keys/rpm.org-rsa-2048-test.secret
 EXTRA_DIST += data/macros.testfile
 EXTRA_DIST += data/macros.debug
+EXTRA_DIST += data/SOURCES/foo.c
+EXTRA_DIST += data/SOURCES/bar.c
+EXTRA_DIST += data/SOURCES/baz.c
+EXTRA_DIST += data/SOURCES/foobar.h
 
 .PHONY: populate_testing
 
@@ -129,6 +134,7 @@ atlocal:atlocal.in Makefile
  -e "s,[@]usrbindir[@],$(bindir)," \
  -e "s,[@]usrlibdir[@],$(libdir)," \
  -e "s,[@]execprefix[@],$(exec_prefix)," \
+ -e "s,[@]usrlibexecdir[@],$(rpmlibexecdir),g" \
  -e "s,[@]RPMCONFIGDIR[@],$(rpmconfigdir)," \
  -e "s,[@]PYTHON[@],$(PYTHON)," \
< $(srcdir)/atlocal.in > atlocal
@@ -170,7 +176,7 @@ check-local:
 endif !HAVE_FAKECHROOT
 
 installcheck-local: $(check_DATA) populate_testing
-   $(SHELL) '$(TESTSUITE)' AUTOTEST_PATH='$(bindir)' $(TESTSUITEFLAGS)
+   $(SHELL) '$(TESTSUITE)' AUTOTEST_PATH='$(bindir):$(rpmlibexecdir)' 
$(TESTSUITEFLAGS)
HOME=$(abs_builddir)/testing $(KILLAGENT) ||:
 
 clean-local:
diff --git a/tests/atlocal.in b/tests/atlocal.in
index fcbf783..0974693 100644
--- a/tests/atlocal.in
+++ b/tests/atlocal.in
@@ -1,6 +1,6 @@
 LD_LIBRARY_PATH="${abs_builddir}/testing@usrlibdir@"
 export LD_LIBRARY_PATH
-PATH="${abs_builddir}/testing@rpmbindir@:${abs_builddir}/testing@usrbindir@:$PATH"
+PATH="${abs_builddir}/testing@rpmbindir@:${abs_builddir}/testing@usrbindir@:${abs_builddir}/testing@usrlibexecdir@:$PATH"
 export PATH
 
 PYTHON=@PYTHON@
diff --git a/tests/data/SOURCES/bar.c b/tests/data/SOURCES/bar.c
new file mode 100644
index 000..6d67469
--- /dev/null
+++ b/tests/data/SOURCES/bar.c
@@ -0,0 +1,9 @@
+#include "foobar.h"
+
+int number = NUMBER;
+
+int
+bar (int n)
+{
+  return n - NUMBER;
+}
diff --git a/tests/data/SOURCES/baz.c b/tests/data/SOURCES/baz.c
new file mode 100644
index 000..4723022
--- /dev/null
+++ b/tests/data/SOURCES/baz.c
@@ -0,0 +1,10 @@
+#include "foobar.h"
+
+int
+main ()
+{
+  int res;
+  res = foo ();
+  res = bar (res);
+  return res + number - NUMBER;
+}
diff --git a/tests/data/SOURCES/foo.c b/tests/data/SOURCES/foo.c
new file mode 100644
index 000..57906a2
--- /dev/null
+++ b/tests/data/SOURCES/foo.c
@@ -0,0 +1,7 @@
+#include "foobar.h"
+
+int
+foo ()
+{
+  return number;
+}
diff --git a/tests/data/SOURCES/foobar.h b/tests/data/SOURCES/foobar.h
new file mode 100644
index 000..6db49a8
--- /dev/null
+++ b/tests/data/SOURCES/foobar.h
@@ -0,0 +1,6 @@
+#define NUMBER 42
+
+extern int number;
+
+extern int foo (void);
+extern int bar (int);
diff --git a/tests/debugedit.at b/tests/debugedit.at
new file mode 100644
index 000..8b724d3
--- /dev/null
+++ b/tests/debugedit.at
@@ -0,0 +1,322 @@
+# debugedit.at: Tests for the debugedit tool
+#
+# Copyright (C) 2019 Mark J. Wielaard 
+#
+# This program 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 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed 

[Rpm-maint] [PATCH 3/5] debugedit: Refactor reading/writing of relocated values.

[Mark Wielaard]

This refactors the reading and writing of relocated values into seperate
helper functions (setup_relbuf and update_rela_data). It will be easier
to reuse this code in case we want to read/write relocated values in other
sections than DEBUG_INFO. The only functional change is that we explicitly
track whether the relocation data is updated, and only explicitly update
and write out the relocation data if so. In the case there were no strp
or stmt updates, there will also not be any relocation updates, even if
there is relocation data available.

All new debugedit testcases pass before and after this refactoring.
---
 tools/debugedit.c | 395 +-
 1 file changed, 216 insertions(+), 179 deletions(-)

diff --git a/tools/debugedit.c b/tools/debugedit.c
index 4be85b9..cf9cc3c 100644
--- a/tools/debugedit.c
+++ b/tools/debugedit.c
@@ -401,13 +401,18 @@ dwarf2_write_be32 (unsigned char *p, uint32_t v)
relend). Might just update the addend. So relocations need to be
updated at the end.  */
 
+static bool rel_updated;
+
 #define do_write_32_relocated(ptr,val) ({ \
   if (relptr && relptr < relend && relptr->ptr == ptr) \
 {  \
   if (reltype == SHT_REL)  \
do_write_32 (ptr, val - relptr->addend);\
   else \
-   relptr->addend = val;   \
+   {   \
+ relptr->addend = val; \
+ rel_updated = true;   \
+   }   \
 }  \
   else \
 do_write_32 (ptr,val); \
@@ -418,14 +423,18 @@ dwarf2_write_be32 (unsigned char *p, uint32_t v)
   ptr += 4;   \
 })
 
-static struct
+typedef struct debug_section
   {
 const char *name;
 unsigned char *data;
 Elf_Data *elf_data;
 size_t size;
 int sec, relsec;
-  } debug_sections[] =
+REL *relbuf;
+REL *relend;
+  } debug_section;
+
+static debug_section debug_sections[] =
   {
 #define DEBUG_INFO 0
 #define DEBUG_ABBREV   1
@@ -458,6 +467,201 @@ static struct
 { NULL, NULL, NULL, 0, 0, 0 }
   };
 
+static int
+rel_cmp (const void *a, const void *b)
+{
+  REL *rela = (REL *) a, *relb = (REL *) b;
+
+  if (rela->ptr < relb->ptr)
+return -1;
+
+  if (rela->ptr > relb->ptr)
+return 1;
+
+  return 0;
+}
+
+/* Returns a malloced REL array, or NULL when there are no relocations
+   for this section.  When there are relocations, will setup relend,
+   as the last REL, and reltype, as SHT_REL or SHT_RELA.  */
+static void
+setup_relbuf (DSO *dso, debug_section *sec, int *reltype)
+{
+  int ndx, maxndx;
+  GElf_Rel rel;
+  GElf_Rela rela;
+  GElf_Sym sym;
+  GElf_Addr base = dso->shdr[sec->sec].sh_addr;
+  Elf_Data *symdata = NULL;
+  int rtype;
+  REL *relbuf;
+  Elf_Scn *scn;
+  Elf_Data *data;
+  int i = sec->relsec;
+
+  /* No relocations, or did we do this already? */
+  if (i == 0 || sec->relbuf != NULL)
+{
+  relptr = sec->relbuf;
+  relend = sec->relend;
+  return;
+}
+
+  scn = dso->scn[i];
+  data = elf_getdata (scn, NULL);
+  assert (data != NULL && data->d_buf != NULL);
+  assert (elf_getdata (scn, data) == NULL);
+  assert (data->d_off == 0);
+  assert (data->d_size == dso->shdr[i].sh_size);
+  maxndx = dso->shdr[i].sh_size / dso->shdr[i].sh_entsize;
+  relbuf = malloc (maxndx * sizeof (REL));
+  *reltype = dso->shdr[i].sh_type;
+  if (relbuf == NULL)
+error (1, errno, "%s: Could not allocate memory", dso->filename);
+
+  symdata = elf_getdata (dso->scn[dso->shdr[i].sh_link], NULL);
+  assert (symdata != NULL && symdata->d_buf != NULL);
+  assert (elf_getdata (dso->scn[dso->shdr[i].sh_link], symdata) == NULL);
+  assert (symdata->d_off == 0);
+  assert (symdata->d_size == dso->shdr[dso->shdr[i].sh_link].sh_size);
+
+  for (ndx = 0, relend = relbuf; ndx < maxndx; ++ndx)
+{
+  if (dso->shdr[i].sh_type == SHT_REL)
+   {
+ gelf_getrel (data, ndx, );
+ rela.r_offset = rel.r_offset;
+ rela.r_info = rel.r_info;
+ rela.r_addend = 0;
+   }
+  else
+   gelf_getrela (data, ndx, );
+  gelf_getsym (symdata, ELF64_R_SYM (rela.r_info), );
+  /* Relocations against section symbols are uninteresting in REL.  */
+  if (dso->shdr[i].sh_type == SHT_REL && sym.st_value == 0)
+   continue;
+  /* Only consider relocations against .debug_str, .debug_line
+and .debug_abbrev.  */
+  if (sym.st_shndx != debug_sections[DEBUG_STR].sec
+ && sym.st_shndx != debug_sections[DEBUG_LINE].sec
+ && sym.st_shndx != debug_sections[DEBUG_ABBREV].sec)
+   continue;
+  rela.r_addend += sym.st_value;
+  rtype = 

[Rpm-maint] [PATCH 1/2] Fix --without-lua build.

[Mark Wielaard]

commit 62bd62 "Add a rpmlua wrapper function for registering libraries"
moved the rpmluaRegister function definition into rpmio/rpmlua.h and
added an #include . This made that file not compile anymore
when lua-devel isn't installed and configuring --without-lua. Even though
that function isn't compiled or used when not using lua with rpm.

Fix this by wrapping both the include and the function definition
in #ifdef WITH_LUA.
---
 rpmio/rpmlua.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/rpmio/rpmlua.h b/rpmio/rpmlua.h
index 9179b37..87418c6 100644
--- a/rpmio/rpmlua.h
+++ b/rpmio/rpmlua.h
@@ -1,7 +1,9 @@
 #ifndef RPMLUA_H
 #define RPMLUA_H
 
+#ifdef WITH_LUA
 #include 
+#endif
 
 typedef enum rpmluavType_e {
 RPMLUAV_NIL= 0,
@@ -20,7 +22,10 @@ extern "C" {
 rpmlua rpmluaNew(void);
 rpmlua rpmluaFree(rpmlua lua);
 rpmlua rpmluaGetGlobalState(void);
+
+#ifdef WITH_LUA
 void rpmluaRegister(rpmlua lua, const luaL_Reg *funcs, const char *lib);
+#endif
 
 int rpmluaCheckScript(rpmlua lua, const char *script,
  const char *name);
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 2/2] Skip tests that use lua (indirectly) when configured --without-lua.

[Mark Wielaard]

The rpmbuild.at autosetup test uses lua indirectly and the rpmscript.at
basic script failures 1 testcase uses lua directly. Skip both when
lua isn't available.

With this patch and configure --without-lua the test results will be:

383 tests behaved as expected.
113 tests were skipped.
---
 tests/rpmbuild.at  | 1 +
 tests/rpmscript.at | 1 +
 2 files changed, 2 insertions(+)

diff --git a/tests/rpmbuild.at b/tests/rpmbuild.at
index 8120cab..bcde532 100644
--- a/tests/rpmbuild.at
+++ b/tests/rpmbuild.at
@@ -38,6 +38,7 @@ AT_CLEANUP
 
 AT_SETUP([rpmbuild -ba autosetup])
 AT_KEYWORDS([build])
+AT_SKIP_IF([$LUA_DISABLED])
 AT_CHECK([
 rm -rf ${TOPDIR}
 AS_MKDIR_P(${TOPDIR}/SOURCES)
diff --git a/tests/rpmscript.at b/tests/rpmscript.at
index ac6c76d..aacc448 100644
--- a/tests/rpmscript.at
+++ b/tests/rpmscript.at
@@ -4,6 +4,7 @@ AT_BANNER([RPM scriptlets])
 
 AT_SETUP([basic script failures 1])
 AT_KEYWORDS([script])
+AT_SKIP_IF([$LUA_DISABLED])
 AT_CHECK([
 RPMDB_CLEAR
 RPMDB_INIT
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] debugedit fails to update section headers (#423)

[Mark Wielaard]

On Mon, Apr 29, 2019 at 02:24:33PM +, Michael Schroeder wrote:
> Mark, any update on this? We (SUSE) get corrupt debuginfo files from
> time to time because of this bug.

Apologies for the slow response. I looked at it before, but didn't
understand what was going wrong.  But I now believe this is actually
an elfutils libelf issue:
https://sourceware.org/ml/elfutils-devel/2019-q2/msg00077.html

Could you try that out and see if it resolves the issue for you?
(The actual patch is just 3 lines, but there is also a larger testcase
added to make sure the issue was really resolved.)

Thanks,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] RFE: provide a mecanism to store buildroot composition in the srpm or a specific subpackage (#607)

[Mark Wielaard]

On Wed, May 01, 2019 at 12:56:39AM -0700, nim-nim wrote:
> It turns out that even an historic package like elfutils has to hack this in 
> `%check` because it's not done by default.

I was wondering what this was referring to and I think you mean:

%check
# Record some build root versions in build.log
uname -r; rpm -q glibc

It is indeed useful to know the kernel (and glibc) version that the
elfutils testsuite is been run against. Especially since it contains
various testcases for core dumps as produced by the kernel, which
contain CFI as produced by the glibc version.

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Add flag to use strip -g instead of full strip on DSOs (RhBug:1663264) (#643)

[Mark Wielaard]

On Tue, 2019-03-19 at 12:30 +, pavlinamv wrote:
> Thank you very much for the review. According of the review I added
> changes in two places:
> -  in documentation: explicitly say "ONLY on DSOs" to make clear how
> it differs from -g. (as Mark wrote) and add that options -g and --g-
> libs are mutually exclusive)
>  - add an error if the user gives both -g and -g-libs.

This version looks perfectly fine to me.

Thanks,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Add flag to use strip -g instead of full strip on DSOs (RhBug:1663264) (#643)

[Mark Wielaard]

On Sat, 2019-03-16 at 22:56 -0700, pavlinamv wrote:
> From 960860164a3c5277d3a31cbf73e60c1d4a16 Mon Sep 17 00:00:00 2001
> From: Pavlina Moravcova Varekova 
> Date: Sun, 17 Mar 2019 06:47:26 +0100
> Subject: [PATCH] Add flag to use strip -g instead of full strip on DSOs
>  (RhBug:1663264)
> 
> The find-debuginfo.sh flag -g had exactly this meaning. But from
> version rpm-4.13.0-alpha flag -g changes its behavior. It affects
> both libraries and executables.
> 
> For some packages the original behavior was preferred. That is why
> the new find-debuginfo.sh flag --g-libs is created.

I think the patch is correct.

There is just one little issue. That might just be solved with an extra
line of documentation. It isn't immediately clear if the intention is
that -g and --g-libs are used together or not. As mentioned below it
would be good to document the meaning of given only -g, only --g-libs
or -g and --g-libs together.

> ---
>  scripts/find-debuginfo.sh | 17 -
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/find-debuginfo.sh b/scripts/find-debuginfo.sh
> index 6e3ba2ce0..285ee51db 100755
> --- a/scripts/find-debuginfo.sh
> +++ b/scripts/find-debuginfo.sh
> @@ -4,6 +4,7 @@
>  #
>  # Usage: find-debuginfo.sh [--strict-build-id] [-g] [-r] [-m] [-i] [-n]
>  #   [--keep-section SECTION] [--remove-section SECTION]
> +#   [--g-libs]
>  #   [-j N] [--jobs N]
>  #   [-o debugfiles.list]
>  #   [-S debugsourcefiles.list]
> @@ -16,6 +17,7 @@
>  #   [builddir]
>  #
>  # The -g flag says to use strip -g instead of full strip on DSOs or EXEs.
> +# The --g-libs flag says to use strip -g instead of full strip on DSOs.

I would explicitly say "ONLY on DSOs" to make clear how it differs from
-g. Assuming the user is intended to only use either "full" -g or
"library-only" --g-libs.

> @@ -68,6 +70,9 @@ lib_rpm_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && 
> pwd )"
>  # With -g arg, pass it to strip on libraries or executables.
>  strip_g=false
>  
> +# With --g-libs arg, pass it to strip on libraries.
> +strip_glibs=false
> +
>  # with -r arg, pass --reloc-debug-sections to eu-strip.
>  strip_r=false
>  
> @@ -135,6 +140,9 @@ while [ $# -gt 0 ]; do
>  unique_debug_src_base=$2
>  shift
>  ;;
> +  --g-libs)
> +strip_glibs=true
> +;;
>-g)
>  strip_g=true
>  ;;

What do we want to do when the user gives both -g and -g-libs together?
Maybe we want to warn (or error) if the user gives both -g and -g-libs. 
Since it isn't completely clear which functionality the user is
requesting.

Alternatively since -g-libs is a subset of "full" -g we might simply
set strip_g=true if just -g is given. That might need a code tweak
below though.

But whatever choice we make, it should be documented.

> @@ -237,6 +245,9 @@ strip_to_debug()
>application/x-executable*) g=-g ;;
>application/x-pie-executable*) g=-g ;;
>esac
> +  $strip_glibs && case "$(file -bi "$2")" in
> +application/x-sharedlib*) g=-g ;;
> +  esac
>eu-strip --remove-comment $r $g ${keep_remove_args} -f "$1" "$2" || exit
>chmod 444 "$1" || exit
>  }

That looks correct.

> @@ -409,8 +420,12 @@ do_file()
># libraries. Other executable ELF files (like kernel modules) don't need 
> it.
>if [ "$include_minidebug" = "true" -a "$strip_g" = "false" ]; then
>  skip_mini=true
> +if [ "$strip_glibs" = "false" ]; then
> +  case "$(file -bi "$f")" in
> +application/x-sharedlib*) skip_mini=false ;;
> +  esac
> +fi
>  case "$(file -bi "$f")" in
> -  application/x-sharedlib*) skip_mini=false ;;
>application/x-executable*) skip_mini=false ;;
>application/x-pie-executable*) skip_mini=false ;;
>  esac

This works correctly, if -g and --g-libs are used exclusively.
But when used together it might depend on how we interpret that
situation.

Cheers,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Add flag to use strip -g instead of full strip on DSOs (RhBug:1663264) (#643)

[Mark Wielaard]

On Mon, 2019-03-18 at 15:40 +0100, Mark Wielaard wrote:
> Alternatively since -g-libs is a subset of "full" -g we might simply
> set strip_g=true if just -g is given. That might need a code tweak
> below though.

boo. now I am confusing myself... sorry.
I meant: "we might simply set strip_glibs=true if just -g is given."

Cheers,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] rpmbuild: File listed twice (but only build-ids)

[Mark Wielaard]

On Tue, 2019-01-15 at 10:40 -0600, Richard Shaw wrote:
> Ooooaaa...
> 
> So this has been around since at least 2017 and there's no fix?

Much longer than that. At least since 2012, probably earlier.
%exclude is discouraged, which is why it doesn't seem to have higher
priority. See also:
https://bugzilla.redhat.com/show_bug.cgi?id=878863
(Although that bug is a little confusing since it seems to mix up two
issues with exclude files and build-id symlinks.)

It is a variant of:
https://github.com/rpm-software-management/rpm/issues/284
The fix for that excludes debug files for files which were excluded in
the non-debug package.

A similar fix should be done for generating the build-ids only for
binaries not in the pkg->fileExcludeList. Currently the
generateBuildIDs () function in build/files.c only gets the full
package file list. It probably should also get the fileExcludeList and
only generate the build-id links for files not on that list.

The interaction between the different package lists is a little
confusing though.

It is probably best to ask for guidance on the upstream mailinglist
rpm-maint@lists.rpm.org (CCed).

> Is there an option to make it a warning again instead of an error?

I don't think without disabling build-ids completely.

Cheers,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [PATCH] find-debuginfo.sh: Handle application/x-pie-executable (file 5.33 output).

[Mark Wielaard]

On Tue, 2018-05-22 at 22:20 +0200, Mark Wielaard wrote:
> A new version of file (5.33) changed the output for PIE executables.
> Which are now represented as application/x-pie-executable;
> charset=binary.
> 
> The following change simply recognizes that binary format as one for
> which -g applies. This fixes accidental stripping of the .symtab in
> glibc (which use -g to keep that symbol table).
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1581224

Ping. Fedora has since backed out the upstream change to file which
caused this. But the patch is still useful/necessary when using a
newer/upstream file.

> Signed-off-by: Mark Wielaard 
> ---
>  scripts/find-debuginfo.sh | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/scripts/find-debuginfo.sh b/scripts/find-debuginfo.sh
> index a7c2db0..90a4494 100755
> --- a/scripts/find-debuginfo.sh
> +++ b/scripts/find-debuginfo.sh
> @@ -235,6 +235,7 @@ strip_to_debug()
>    $strip_g && case "$(file -bi "$2")" in
>    application/x-sharedlib*) g=-g ;;
>    application/x-executable*) g=-g ;;
> +  application/x-pie-executable*) g=-g ;;
>    esac
>    eu-strip --remove-comment $r $g ${keep_remove_args} -f "$1" "$2"
> || exit
>    chmod 444 "$1" || exit
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] find-debuginfo.sh: Handle application/x-pie-executable (file 5.33 output).

[Mark Wielaard]

A new version of file (5.33) changed the output for PIE executables.
Which are now represented as application/x-pie-executable; charset=binary.

The following change simply recognizes that binary format as one for
which -g applies. This fixes accidental stripping of the .symtab in
glibc (which use -g to keep that symbol table).

https://bugzilla.redhat.com/show_bug.cgi?id=1581224

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 scripts/find-debuginfo.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/find-debuginfo.sh b/scripts/find-debuginfo.sh
index a7c2db0..90a4494 100755
--- a/scripts/find-debuginfo.sh
+++ b/scripts/find-debuginfo.sh
@@ -235,6 +235,7 @@ strip_to_debug()
   $strip_g && case "$(file -bi "$2")" in
   application/x-sharedlib*) g=-g ;;
   application/x-executable*) g=-g ;;
+  application/x-pie-executable*) g=-g ;;
   esac
   eu-strip --remove-comment $r $g ${keep_remove_args} -f "$1" "$2" || exit
   chmod 444 "$1" || exit
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [PATCH] debugedit: Check .debug_str index is valid before use.

[Mark Wielaard]

Hi Panu,

On Mon, 2018-03-12 at 14:07 +0200, Panu Matilainen wrote:
> Actually, this breaks a bunch of testcases:
> 
> 139: rpmbuild debuginfo subpackages multiple FAILED 
> (rpmbuild.at:973)
> 140: rpmbuild debuginfo subpackages multiple unique  FAILED 
> (rpmbuild.at:1058)
> 141: rpmbuild debuginfo subpackages multiple unique debugsource FAILED 
> (rpmbuild.at:1143)
> 142: rpmbuild debuginfo subpackages multiple excluded FAILED 
> (rpmbuild.at:1231)
> 143: rpmbuild debuginfo subpackages multiple excluded FAILED 
> (rpmbuild.at:1296)
> 
> They're all failing with messages like this:
> 
> /home/pmatilai/repos/rpm/tests/testing/usr/lib/rpm/debugedit: 
> /home/pmatilai/repos/rpm/tests/testing/build/BUILDROOT/test-1.0-1.x86_64/bin/hello2:
>  
> Bad string pointer index 678 for unit name

That is embarrassing. I was sure I got zero FAIL on the testsuite
before sending the patch. But now I see the same (on Fedora 27).

It might be because ./tests/testing/usr/lib/rpm/debugedit doesn't seem
to be regenerated by the build. So you only see it on a fully fresh
build, not on an incremental one.

This is an interesting bug, because it was latent. We were already
accessing a bad string before, but for some reason we got away with it.

This code walks over the DIE tree in two phases. In phase zero we want
to collect the comp_dir (either from the DW_AT_comp_dir or the
DW_AT_name of the compile unit). The bug happens in phase 1, when we
don't need to collect the comp_dir. But we still do, wrongly using the
old string table... So the error is kind of correct.

Fix attached, which looks a bit funny, but really only wraps the
affected code in an if (phase == 0) block.

Cheers,

MarkFrom 8e6eb541067da7848b600b95c6ff8c3bad5c1f0d Mon Sep 17 00:00:00 2001
From: Mark Wielaard <m...@klomp.org>
Date: Mon, 12 Mar 2018 14:16:15 +0100
Subject: [PATCH] debugedit: Only try to collect comp_dir in phase zero.

edit_attributes is run twice. Once for phase zero in which all strings are
collected. Then then for phase one in which the strings are rewritten. In
phase zero we also try to collect the comp_dir (either from the
DW_AT_comp_dir or the DW_AT_name of the compile unit). We were also
collecting the comp_dir is phase 1, which is unnecessary, and would not
actually work, since we would be using to old string table index for that,
which had already been rewritten.

Caught by the new string table index checks.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 tools/debugedit.c | 52 ++--
 1 file changed, 30 insertions(+), 22 deletions(-)

diff --git a/tools/debugedit.c b/tools/debugedit.c
index 6c71cbcbe..84568dd29 100644
--- a/tools/debugedit.c
+++ b/tools/debugedit.c
@@ -1539,14 +1539,18 @@ edit_attributes (DSO *dso, unsigned char *ptr, struct abbrev_tag *t, int phase)
 		{
 		  const char *dir;
 		  size_t idx = do_read_32_relocated (ptr);
-		  if (idx >= debug_sections[DEBUG_STR].size)
-		error (1, 0,
-			   "%s: Bad string pointer index %zd for comp_dir",
-			   dso->filename, idx);
-		  dir = (char *) debug_sections[DEBUG_STR].data + idx;
+		  /* In phase zero we collect the comp_dir.  */
+		  if (phase == 0)
+		{
+		  if (idx >= debug_sections[DEBUG_STR].size)
+			error (1, 0,
+			   "%s: Bad string pointer index %zd for comp_dir",
+			   dso->filename, idx);
+		  dir = (char *) debug_sections[DEBUG_STR].data + idx;
 
-		  free (comp_dir);
-		  comp_dir = strdup (dir);
+		  free (comp_dir);
+		  comp_dir = strdup (dir);
+		}
 
 		  if (dest_dir != NULL && phase == 0)
 		{
@@ -1566,25 +1570,29 @@ edit_attributes (DSO *dso, unsigned char *ptr, struct abbrev_tag *t, int phase)
 		 unit. If starting with / it is a full path name.
 		 Note that we don't handle DW_FORM_string in this
 		 case.  */
-	  char *name;
 	  size_t idx = do_read_32_relocated (ptr);
-	  if (idx >= debug_sections[DEBUG_STR].size)
-		error (1, 0,
-		   "%s: Bad string pointer index %zd for unit name",
-		   dso->filename, idx);
-	  name = (char *) debug_sections[DEBUG_STR].data + idx;
-	  if (*name == '/' && comp_dir == NULL)
-		{
-		  char *enddir = strrchr (name, '/');
 
-		  if (enddir != name)
+	  /* In phase zero we will look for a comp_dir to use.  */
+	  if (phase == 0)
+		{
+		  if (idx >= debug_sections[DEBUG_STR].size)
+		error (1, 0,
+			   "%s: Bad string pointer index %zd for unit name",
+			   dso->filename, idx);
+		  char *name = (char *) debug_sections[DEBUG_STR].data + idx;
+		  if (*name == '/' && comp_dir == NULL)
 		{
-		  comp_dir = malloc (enddir - name + 1);
-		  memcpy (comp_dir, name, enddir - name);
-		  comp_dir [enddir - name] = '\0';
+		  char *enddir = strrchr (name, '/');
+
+		  if (enddir != name)
+			{
+			  comp_dir

[Rpm-maint] [PATCH] debugedit: Check .debug_str index is valid before use.

[Mark Wielaard]

debugedit would blindly use an .debug_str index from the .debug_info or
.debug_line sections assuming it would result in a valid string. Which
would crash and burn if the DWARF data was bogus when the string was
used. So check whenever converting an string index into a char pointer
so we can produce a more helpful error message.

https://bugzilla.redhat.com/show_bug.cgi?id=1543912

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 tools/debugedit.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/tools/debugedit.c b/tools/debugedit.c
index 57cd830..6c71cbc 100644
--- a/tools/debugedit.c
+++ b/tools/debugedit.c
@@ -820,6 +820,9 @@ record_file_string_entry_idx (struct strings *strings, 
size_t old_idx)
   struct stridxentry *entry = string_find_new_entry (strings, old_idx);
   if (entry != NULL)
 {
+  if (old_idx >= debug_sections[DEBUG_STR].size)
+   error (1, 0, "Bad string pointer index %zd", old_idx);
+
   Strent *strent;
   const char *old_str = (char *)debug_sections[DEBUG_STR].data + old_idx;
   const char *file = skip_dir_prefix (old_str, base_dir);
@@ -870,6 +873,9 @@ record_existing_string_entry_idx (struct strings *strings, 
size_t old_idx)
   struct stridxentry *entry = string_find_new_entry (strings, old_idx);
   if (entry != NULL)
 {
+  if (old_idx >= debug_sections[DEBUG_STR].size)
+   error (1, 0, "Bad string pointer index %zd", old_idx);
+
   const char *str = (char *)debug_sections[DEBUG_STR].data + old_idx;
   Strent *strent = strtab_add_len (strings->str_tab,
   str, strlen (str) + 1);
@@ -1533,6 +1539,10 @@ edit_attributes (DSO *dso, unsigned char *ptr, struct 
abbrev_tag *t, int phase)
{
  const char *dir;
  size_t idx = do_read_32_relocated (ptr);
+ if (idx >= debug_sections[DEBUG_STR].size)
+   error (1, 0,
+  "%s: Bad string pointer index %zd for comp_dir",
+  dso->filename, idx);
  dir = (char *) debug_sections[DEBUG_STR].data + idx;
 
  free (comp_dir);
@@ -1558,6 +1568,10 @@ edit_attributes (DSO *dso, unsigned char *ptr, struct 
abbrev_tag *t, int phase)
 case.  */
  char *name;
  size_t idx = do_read_32_relocated (ptr);
+ if (idx >= debug_sections[DEBUG_STR].size)
+   error (1, 0,
+  "%s: Bad string pointer index %zd for unit name",
+  dso->filename, idx);
  name = (char *) debug_sections[DEBUG_STR].data + idx;
  if (*name == '/' && comp_dir == NULL)
{
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] Fix type mismatch calculating new line program offset in debugedit.c.

[Mark Wielaard]

edit_dwarf2 calculates the (new) offset in the line program by
taking the difference between the old and new idx, which are of type
size_t (unsigned), plus the size_diff of the header given as ssize_t
(signed), and adding that to the current r_offset, which is an Elf64_Addr
(unsigned). On 64bit architectures, where the size of Elf64_Addr and
ssize_t are the same this isn't a problem. But on 32bit architectures,
where the size of ssize_t is smaller than Elf64_Addr the smaller signed
result gets promoted to an unsigned long first causing issues if the
size_diff was negative.

This would have been caught by gcc -Wsign-conversion

warning: conversion to ‘long unsigned int’ from ‘ssize_t’ {aka ‘long int’}
may change the sign of the result

But enabling this by default gives a lot of false positives.

Found and fixed by Richard Biener .
---
 tools/debugedit.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/debugedit.c b/tools/debugedit.c
index fa47aa5..e0b1d98 100644
--- a/tools/debugedit.c
+++ b/tools/debugedit.c
@@ -2154,9 +2154,9 @@ edit_dwarf2 (DSO *dso)
  /* Offset (pointing into the line program) moves
 from old to new index including the header
 size diff. */
- r_offset += ((dso->lines.table[lndx].new_idx
-   - dso->lines.table[lndx].old_idx)
-  + dso->lines.table[lndx].size_diff);
+ r_offset += (ssize_t)((dso->lines.table[lndx].new_idx
+- dso->lines.table[lndx].old_idx)
+   + dso->lines.table[lndx].size_diff);
 
  if (rtype == SHT_RELA)
{
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Missing build id, Generating build-id links failed (#367)

[Mark Wielaard]

On Sun, Dec 10, 2017 at 09:34:25AM +0100, Tzafrir Cohen wrote:
> On Fri, Dec 08, 2017 at 12:35:44AM +0100, Mark Wielaard wrote:
> > At least the Fedora go toolchain should be able to generate proper 
> > build-ids these days.
> > If not, see these (old) notes:
> > https://fedoraproject.org/wiki/PackagingDrafts/Go#Build_ID
> 
> So is the official answer to use gccgo?

I don't know if that is an official answer, but gccgo certainly produces
more conforming ELF objects.

> I recently ran into this issue and found no good answer. I ended up
> using the following Makefile snippet:
> 
> BUILD_ID=$(shell head -c20 /dev/urandom|od -An -tx1|tr -d ' \n')
> BUILD_ID_OPT=-B 0x$(BUILD_ID)
> BUILD_ARGS=-ldflags "-X main.version=$(VERSION) -X main.commit=$(COMMIT) -X 
> main.buildtime=$(BUILD_TIME) $(BUILD_ID_OPT)"
> 
> (Yes, not a reproducible build if I use buildtime, but useful so far)

I believe that is what Fedora also uses. Or a variant that is reproducible.

You can also of course undefine the _missing_build_ids_terminate_build macro
in your spec file to work around such toolchain issues.

Cheers,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Missing build id, Generating build-id links failed (#367)

[Mark Wielaard]

Hi Eli,

On Thu, Dec 07, 2017 at 03:20:01PM -0800, Eli Uriegas wrote:
> ```
> error: Missing build-id in 
> /root/rpmbuild/BUILDROOT/docker-ce-17.12.0.ce-0.1.rc1.fc27.x86_64/usr/bin/docker-containerd-shim
> error: Generating build-id links failed
> Missing build-id in 
> /root/rpmbuild/BUILDROOT/docker-ce-17.12.0.ce-0.1.rc1.fc27.x86_64/usr/bin/docker-containerd-shim
> Generating build-id links failed
> ```

That probably means the compiler/linker of that binary didn't generate a proper
build-id for that binary. The golang toolchain didn't in the past.

> I believe it may have something to do with this commit: 
> https://github.com/rpm-software-management/rpm/commit/5e82c7e8a8fc05195cdf622d0a120b9e70a9371b

Why do you believe that? That commit should only fix issues for noarch packages.
Real binaries must have build-ids.

At least the Fedora go toolchain should be able to generate proper build-ids 
these days.
If not, see these (old) notes:
https://fedoraproject.org/wiki/PackagingDrafts/Go#Build_ID

Cheers,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] Drowning in build-ids

[Mark Wielaard]

On Wed, 2017-10-25 at 12:49 +0200, Igor Gnatenko wrote:
> On Wed, 2017-10-25 at 13:46 +0300, Panu Matilainen wrote:
> So I'm wondering how to make this less ugly.
> > 
> > The first thing that comes to mind is adding a %hidden virtual
> > attribute 
> > and using it on build-ids (which are in a hidden directory on the 
> > filesystem), which would hide such files rpm -ql etc output by
> > default 
> > (but with a cli-switch to show it all).
> > 
> > Another option would be hiding files and directories starting with
> > dot, 
> > ie mirror the filesystem behavior. Obviously with a switch to show
> > them too.
> > 
> > The idea of being able to hide arbitrary files from default output
> > makes 
> > me a bit queasy. And also %hidden wouldn't help with existing
> > packages, 
> > (mass) rebuilds are needed with that option. So it seems like two
> > points 
> > in favor of the fs behavior, but dunno.
> > 
> > Thoughts, comments, better ideas?
> 
> I definitely like FS approach (2). But also having %hidden (1) would
> find its use I think.

I don't like the name %hidden, but I think that having an official
attribute like "%artificial" might be the correct way to go. Then any
file added by rpm/file trigger/etc that wasn't explicitly mentioned in
the spec %files list could get that attribute. If you have that then
you can have a rpm -qA to list all "artificial" files of the rpm (and
rpm -qlA would show all).

I don't like the hide .dot files heuristic. People might have
explicitly added .dot files to their spec %files. Then I think they
should be shown by default I think.

But maybe explicitly treat /.build-id/ as artificial and then add an
official %artificial for all "future" use would be a good compromise?

Cheers,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] rpm-4.14.0/tools/debugedit.c:2144]: (style) Array index 'lndx' is used before limits check. (#332)

[Mark Wielaard]

On Fri, Oct 13, 2017 at 07:30:01AM +, dcb314 wrote:
> Source code is
> 
>   while (r_offset > (dso->lines.table[lndx].old_idx
>  + 4
>  + dso->lines.table[lndx].unit_length)
>  && lndx < dso->lines.used)
> lndx++;
> 
> Suggest limit check array index before use, not after.

Yes, nice catch. The conditions should be swapped.
Patch attached.

Cheers,

Mark
>From 3be44f93ab963169b1b1f4f24c7adffb2029f0df Mon Sep 17 00:00:00 2001
From: Mark Wielaard <m...@klomp.org>
Date: Fri, 13 Oct 2017 23:27:32 +0200
Subject: [PATCH] debugedit: edit_dwarf2 check lndx is in range before checking
 r_offset.

---
 tools/debugedit.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/debugedit.c b/tools/debugedit.c
index a271b916e..fa47aa5e2 100644
--- a/tools/debugedit.c
+++ b/tools/debugedit.c
@@ -2141,10 +2141,10 @@ edit_dwarf2 (DSO *dso)
  r_offset = rel.r_offset;
}
 
- while (r_offset > (dso->lines.table[lndx].old_idx
-+ 4
-+ dso->lines.table[lndx].unit_length)
-&& lndx < dso->lines.used)
+ while (lndx < dso->lines.used
+&& r_offset > (dso->lines.table[lndx].old_idx
+   + 4
+   + 
dso->lines.table[lndx].unit_length))
lndx++;
 
  if (lndx >= dso->lines.used)
-- 
2.13.6

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] Fix rpmlog printing of off_t in fdConsume.

[Mark Wielaard]

off_t is a somewhat weird type because the only thing that is defined
about it is that it is signed. Depending on whether you are compiling
on a 32 or 64 system and/or whether compiling for large file support
it has different sizes. Currently compiling build/pack.c on 32bit arches
will give a warning because the size doesn't match the %ld format
specifier. Change it to %jd and explicitly cast it to intmax_t to make
it print consistently on any arch.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 build/pack.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/build/pack.c b/build/pack.c
index 46816cf..1348b5f 100644
--- a/build/pack.c
+++ b/build/pack.c
@@ -431,8 +431,8 @@ static rpmRC fdConsume(FD_t fd, off_t start, off_t nbytes)
 };
 
 if (left) {
-   rpmlog(RPMLOG_ERR, _("Failed to read %ld bytes in file %s: %s\n"),
-   nbytes, Fdescr(fd), Fstrerror(fd));
+   rpmlog(RPMLOG_ERR, _("Failed to read %jd bytes in file %s: %s\n"),
+  (intmax_t) nbytes, Fdescr(fd), Fstrerror(fd));
 }
 
 return (left == 0) ? RPMRC_OK : RPMRC_FAIL;
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] Fix rpmlog warning in lzopen_internal from too big length specifier.

[Mark Wielaard]

memlimit is defined as uint32_t, but was printed as %lu.
Change to %u to avoid a gcc warning.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 rpmio/rpmio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rpmio/rpmio.c b/rpmio/rpmio.c
index d8b8840..c7cbc32 100644
--- a/rpmio/rpmio.c
+++ b/rpmio/rpmio.c
@@ -839,7 +839,7 @@ static LZFILE *lzopen_internal(const char *mode, int fd, 
int xz)
 
if (threads != (int)mt_options.threads)
rpmlog(RPMLOG_NOTICE,
-   "XZ: Adjusted the number of threads from %d to 
%d to not exceed the memory usage limit of %lu bytes",
+   "XZ: Adjusted the number of threads from %d to 
%d to not exceed the memory usage limit of %u bytes",
threads, mt_options.threads, memlimit);
}
 #endif
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] Avoid warning about mbAppendStr if lui support isn't enabled.

[Mark Wielaard]

Guard the whole mbAppendStr function with ifdef WITH_LUA to avoid a
warning when lua support isn't enabled because it is only used inside
the doLua function (which is empty unles WITH_LUA is defined).

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 rpmio/macro.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/rpmio/macro.c b/rpmio/macro.c
index 0f90aaf..b6d8900 100644
--- a/rpmio/macro.c
+++ b/rpmio/macro.c
@@ -394,6 +394,7 @@ static void mbAppend(MacroBuf mb, char c)
 mb->nb--;
 }
 
+#ifdef WITH_LUA
 static void mbAppendStr(MacroBuf mb, const char *str)
 {
 size_t len = strlen(str);
@@ -405,6 +406,8 @@ static void mbAppendStr(MacroBuf mb, const char *str)
 mb->tpos += len;
 mb->nb -= len;
 }
+#endif
+
 /**
  * Expand output of shell command into target buffer.
  * @param mb   macro expansion state
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] rpm4.14 makes perl-RPM4 testsuite to segfault

[Mark Wielaard]

On Thu, 2017-10-05 at 15:36 +0200, Thierry Vignaud wrote:
> On 5 October 2017 at 14:07, Igor Gnatenko
>  wrote:
> > > For the record
> > > BTW, the latest issue I've with rpm-4.14 is that its testsuite
> > > has
> > > several debugsources related failures  when building it with rpm-
> > > 4.14
> > > tpoo
> > > http://pkgsubmit.mageia.org/uploads/done/cauldron/core/release/20
> > > 1710
> > > 05114000.tv.duvel.20120/rpm-4.14.0-
> > > 0.rc2.4.mga7/build.0.20171005114003.log
> > > Have you tried building rpm-4.14 with rpm-4.14?
> > 
> > I think the problem is that you have enabled producing debugsource
> > in
> > RPM while we have it in redhat-rpm-config in Fedora. I've seen same
> > failures when I tried to enable debugsource packages directly in
> > RPM...
> 
> Nope.
> It's not enabled in rpm but it is in the pkg equivalent to
> redhat-rpm-config (rpm-mageia-setup), which is installed by default
> in
> build chroots
> After all we do want rpm-debugsource
> Also di

This sentence doesn't seem finished.

In Fedora rawhide rpm is build with rpm-4.14-rc in the build roots.
It shows zero fails. All 383 tests behaved as expected.
https://kojipkgs.fedoraproject.org//packages/rpm/4.14.0/0.rc2.5.fc28/data/logs/x86_64/build.log
It also generates separate debugsource and debuginfo subpackages for rpm itself.
https://koji.fedoraproject.org/koji/buildinfo?buildID=979215

The tests should enable/disable various debug generation settings
explicitly.

Cheers,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 2/2] Add debugsource recommends to debuginfo packages.

[Mark Wielaard]

Debuginfo packages are useful without debugsource files. But it is often
useful to also have the debugsource files. So make debuginfo packages that
don't contain sources recommend the debugsource package (or the main
debuginfo one if the sources are not in a separate debugsource package).

Add Package dbgsrc as argument to filterDebuginfoPackage so it can be
added as recommendation. Add a new function findDebugsourcePackage.
Use it to add a requires to the main debuginfo file and/or the debuginfo
subpackages.

Extend the various rpmbuild.at tests that create debugsource and/or
debuginfo subpackages to check the debugsource (or main debuginfo)
package is recommended.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 build/files.c | 33 +++--
 tests/rpmbuild.at | 46 +-
 2 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/build/files.c b/build/files.c
index aad9c5e..92f982e 100644
--- a/build/files.c
+++ b/build/files.c
@@ -2773,6 +2773,9 @@ static void patchDebugPackageString(Package dbg, rpmTag 
tag, Package pkg, Packag
 _free(newsubst);
 }
 
+/* Early prototype for use in filterDebuginfoPackage. */
+static void addPackageDeps(Package from, Package to, enum rpmTag_e tag);
+
 /* create a new debuginfo subpackage for package pkg from the
  * main debuginfo package */
 static Package cloneDebuginfoPackage(rpmSpec spec, Package pkg, Package 
maindbg)
@@ -2805,7 +2808,8 @@ static Package cloneDebuginfoPackage(rpmSpec spec, 
Package pkg, Package maindbg)
 /* collect the debug files for package pkg and put them into
  * a (possibly new) debuginfo subpackage */
 static void filterDebuginfoPackage(rpmSpec spec, Package pkg,
-   Package maindbg, char *buildroot, char *uniquearch)
+  Package maindbg, Package dbgsrc,
+  char *buildroot, char *uniquearch)
 {
 rpmfi fi;
 ARGV_t files = NULL;
@@ -2914,6 +2918,9 @@ static void filterDebuginfoPackage(rpmSpec spec, Package 
pkg,
else {
Package dbg = cloneDebuginfoPackage(spec, pkg, maindbg);
dbg->fileList = files;
+   /* Recommend the debugsource package (or the main debuginfo).  */
+   addPackageDeps(dbg, dbgsrc ? dbgsrc : maindbg,
+  RPMTAG_RECOMMENDNAME);
}
 }
 }
@@ -2976,6 +2983,16 @@ static int addDebugSrc(Package pkg, char *buildroot)
 return ret;
 }
 
+/* find the debugsource package, if it has been created.
+ * We do this simply by searching for a package with the right name. */
+static Package findDebugsourcePackage(rpmSpec spec)
+{
+Package pkg = NULL;
+if (lookupPackage(spec, "debugsource", PART_SUBNAME|PART_QUIET, ))
+   return NULL;
+return pkg && pkg->fileList ? pkg : NULL;
+}
+
 /* find the main debuginfo package. We do this simply by
  * searching for a package with the right name. */
 static Package findDebuginfoPackage(rpmSpec spec)
@@ -3009,6 +3026,9 @@ rpmRC processBinaryFiles(rpmSpec spec, rpmBuildPkgFlags 
pkgFlags,
 char *uniquearch = NULL;
 Package maindbg = NULL;/* the (existing) main debuginfo 
package */
 Package deplink = NULL;/* create requires to this package */
+/* The debugsource package, if it exists, that the debuginfo package(s)
+   should Recommend.  */
+Package dbgsrcpkg = findDebugsourcePackage(spec);
 
 #if HAVE_LIBDW
 elf_version (EV_CURRENT);
@@ -3039,6 +3059,12 @@ rpmRC processBinaryFiles(rpmSpec spec, rpmBuildPkgFlags 
pkgFlags,
if (rpmExpandNumeric("%{?_unique_debug_names}"))
uniquearch = rpmExpand("-%{VERSION}-%{RELEASE}.%{_arch}", NULL);
}
+} else if (dbgsrcpkg != NULL) {
+   /* We have a debugsource package, but no debuginfo subpackages.
+  The main debuginfo package should recommend the debugsource one. */
+   Package dbgpkg = findDebuginfoPackage(spec);
+   if (dbgpkg)
+   addPackageDeps(dbgpkg, dbgsrcpkg, RPMTAG_RECOMMENDNAME);
 }
 
 for (pkg = spec->packages; pkg != NULL; pkg = pkg->next) {
@@ -3056,6 +3082,8 @@ rpmRC processBinaryFiles(rpmSpec spec, rpmBuildPkgFlags 
pkgFlags,
deplink = extradbg;
if (addDebugSrc(extradbg, buildroot))
deplink = extradbg;
+   if (dbgsrcpkg != NULL)
+   addPackageDeps(extradbg, dbgsrcpkg, RPMTAG_RECOMMENDNAME);
maindbg = NULL; /* all normal packages processed */
}
 
@@ -3072,7 +3100,8 @@ rpmRC processBinaryFiles(rpmSpec spec, rpmBuildPkgFlags 
pkgFlags,
goto exit;
 
if (maindbg)
-   filterDebuginfoPackage(spec, pkg, maindbg, buildroot, uniquearch);
+   filterDebuginfoPackage(spec, pkg, maindbg, dbgsrcpkg,
+  buildroot, uniquearch);
else if (deplink && pkg != depl

[Rpm-maint] [PATCH 1/2] Rename addPackageRequires to addPackageDeps in build/files.

[Mark Wielaard]

Also add the dependency tag as argument so it can be used to add either
a Requires or Recommends. Adjust the one caller to pass RPMTAG_REQUIRENAME.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 build/files.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/build/files.c b/build/files.c
index 5e84532..aad9c5e 100644
--- a/build/files.c
+++ b/build/files.c
@@ -2986,15 +2986,16 @@ static Package findDebuginfoPackage(rpmSpec spec)
 return pkg && pkg->fileList ? pkg : NULL;
 }
 
-/* add a requires for package "to" into package "from". */
-static void addPackageRequires(Package from, Package to)
+/* add a dependency (e.g. RPMTAG_REQUIRENAME or RPMTAG_RECOMMENDNAME)
+   for package "to" into package "from". */
+static void addPackageDeps(Package from, Package to, enum rpmTag_e tag)
 {
 const char *name;
 char *evr, *isaprov;
 name = headerGetString(to->header, RPMTAG_NAME);
 evr = headerGetAsString(to->header, RPMTAG_EVR);
 isaprov = rpmExpand(name, "%{?_isa}", NULL);
-addReqProv(from, RPMTAG_REQUIRENAME, isaprov, evr, RPMSENSE_EQUAL, 0);
+addReqProv(from, tag, isaprov, evr, RPMSENSE_EQUAL, 0);
 free(isaprov);
 free(evr);
 }
@@ -3073,7 +3074,7 @@ rpmRC processBinaryFiles(rpmSpec spec, rpmBuildPkgFlags 
pkgFlags,
if (maindbg)
filterDebuginfoPackage(spec, pkg, maindbg, buildroot, uniquearch);
else if (deplink && pkg != deplink)
-   addPackageRequires(pkg, deplink);
+   addPackageDeps(pkg, deplink, RPMTAG_REQUIRENAME);
 
 if ((rc = rpmfcGenerateDepends(spec, pkg)) != RPMRC_OK)
goto exit;
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [PATCH] Add debugsource recommends to debuginfo packages.

[Mark Wielaard]

On Thu, 2017-09-21 at 10:41 +0300, Panu Matilainen wrote:
> Moving an entire function while also changing it is a bit of a no-
> no, because it's makes it hard to see what actually changed. Moving
> things around also obfuscates git history, which is why I prefer
> adding a prototype to the top of the file instead. But if you need to
> move stuff around (such as different file entirely), do so in a
> separate commit that doesn't change anything and also state this in
> the commit message.
>
> Also please split refactoring changes like this to separate commits:
> one commit that does the necessary enhancements/changes to 
> addPackageRequires() and updates existing callers, and another commit
> to add the new functionality. It just makes things the diffs so much
> more obvious and nicer to review.

OK. Patch split in two and added an early prototype instead of moving
the function around.

> > @@ -2914,6 +2930,8 @@ static void filterDebuginfoPackage(rpmSpec spec, 
> > Package pkg,
> >     else {
> >     Package dbg = cloneDebuginfoPackage(spec, pkg, maindbg);
> >     dbg->fileList = files;
> > +   /* Recommend the debugsource package (or the main debuginfo).  */
> > +   addPackageRequiresRecommends(dbg, dbgsrc ?: maindbg, true);
> 
> Omitting middle operand of conditional expressions is a gcc
> extension, those are best avoided in general and especially when
> there's no actual need to use one.

I am surprised it is a GNU extension. IMHO it should be standard C. I
like it, because it is more concise and precise than duplicating the
condition. But OK. Changed to the long form.

> [...]
> > +   addPackageRequiresRecommends(pkg, deplink, false);
> 
> Hmm, I think this would be both more obvious and generally useful if
> you just call the function addPackageDeps() and pass the relevant
> dependency tag instead of true/false for require/recommend, eg
> 
> addPackageDeps(pkg, deplink, RPMTAG_REQUIRENAME);
> addPackageDeps(extradbg, dbgsrcpkg, RPMTAG_RECOMMENDNAME);
> 
> vs
> 
> addPackageRequiresRecommends(pkg, deplink, false);
> addPackageRequiresRecommends(extradbg, dbgsrcpkg, true);
> 
> That way we wont end up with 
> addPackageRequiresRecommendsConflictsObsoltetes() one day :)

Makes sense. Changed the function name (and argument).

Thanks,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] Add debugsource recommends to debuginfo packages.

[Mark Wielaard]

Debuginfo packages are useful without debugsource files. But it is often
useful to also have the debugsourc files. So make debuginfo packages that
don't contain sources recommend the debugsource package (or the main
debuginfo one if the sources are not in a separate debugsource package).

Rename addPackageRequires to addPackageRequiresRecommends with an argument
to indicate whether the package is required or recommended. Move up so it
can be used with filterDebuginfoPackage. Add Package dbgsrc as argument to
filterDebuginfoPackage so it can be added as recommendation. Add a new
function findDebugsourcePackage. Use it to add a requires to the main
debuginfo file and/or the debuginfo subpackages.

Extend the various rpmbuild.at tests that create debugsource and/or
debuginfo subpackages to check the debugsource (or main debuginfo)
package is recommended.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 build/files.c | 59 ---
 tests/rpmbuild.at | 46 ++-
 2 files changed, 88 insertions(+), 17 deletions(-)

diff --git a/build/files.c b/build/files.c
index 5e84532..86ec80a 100644
--- a/build/files.c
+++ b/build/files.c
@@ -2773,6 +2773,21 @@ static void patchDebugPackageString(Package dbg, rpmTag 
tag, Package pkg, Packag
 _free(newsubst);
 }
 
+/* add a requires or recommends for package "to" into package "from". */
+static void addPackageRequiresRecommends(Package from, Package to,
+ bool recommends)
+{
+const char *name;
+char *evr, *isaprov;
+enum rpmTag_e tag = recommends ? RPMTAG_RECOMMENDNAME : RPMTAG_REQUIRENAME;
+name = headerGetString(to->header, RPMTAG_NAME);
+evr = headerGetAsString(to->header, RPMTAG_EVR);
+isaprov = rpmExpand(name, "%{?_isa}", NULL);
+addReqProv(from, tag, isaprov, evr, RPMSENSE_EQUAL, 0);
+free(isaprov);
+free(evr);
+}
+
 /* create a new debuginfo subpackage for package pkg from the
  * main debuginfo package */
 static Package cloneDebuginfoPackage(rpmSpec spec, Package pkg, Package 
maindbg)
@@ -2805,7 +2820,8 @@ static Package cloneDebuginfoPackage(rpmSpec spec, 
Package pkg, Package maindbg)
 /* collect the debug files for package pkg and put them into
  * a (possibly new) debuginfo subpackage */
 static void filterDebuginfoPackage(rpmSpec spec, Package pkg,
-   Package maindbg, char *buildroot, char *uniquearch)
+  Package maindbg, Package dbgsrc,
+  char *buildroot, char *uniquearch)
 {
 rpmfi fi;
 ARGV_t files = NULL;
@@ -2914,6 +2930,8 @@ static void filterDebuginfoPackage(rpmSpec spec, Package 
pkg,
else {
Package dbg = cloneDebuginfoPackage(spec, pkg, maindbg);
dbg->fileList = files;
+   /* Recommend the debugsource package (or the main debuginfo).  */
+   addPackageRequiresRecommends(dbg, dbgsrc ?: maindbg, true);
}
 }
 }
@@ -2976,6 +2994,16 @@ static int addDebugSrc(Package pkg, char *buildroot)
 return ret;
 }
 
+/* find the debugsource package, if it has been created.
+ * We do this simply by searching for a package with the right name. */
+static Package findDebugsourcePackage(rpmSpec spec)
+{
+Package pkg = NULL;
+if (lookupPackage(spec, "debugsource", PART_SUBNAME|PART_QUIET, ))
+   return NULL;
+return pkg && pkg->fileList ? pkg : NULL;
+}
+
 /* find the main debuginfo package. We do this simply by
  * searching for a package with the right name. */
 static Package findDebuginfoPackage(rpmSpec spec)
@@ -2986,19 +3014,6 @@ static Package findDebuginfoPackage(rpmSpec spec)
 return pkg && pkg->fileList ? pkg : NULL;
 }
 
-/* add a requires for package "to" into package "from". */
-static void addPackageRequires(Package from, Package to)
-{
-const char *name;
-char *evr, *isaprov;
-name = headerGetString(to->header, RPMTAG_NAME);
-evr = headerGetAsString(to->header, RPMTAG_EVR);
-isaprov = rpmExpand(name, "%{?_isa}", NULL);
-addReqProv(from, RPMTAG_REQUIRENAME, isaprov, evr, RPMSENSE_EQUAL, 0);
-free(isaprov);
-free(evr);
-}
-
 rpmRC processBinaryFiles(rpmSpec spec, rpmBuildPkgFlags pkgFlags,
int didInstall, int test)
 {
@@ -3008,6 +3023,9 @@ rpmRC processBinaryFiles(rpmSpec spec, rpmBuildPkgFlags 
pkgFlags,
 char *uniquearch = NULL;
 Package maindbg = NULL;/* the (existing) main debuginfo 
package */
 Package deplink = NULL;/* create requires to this package */
+/* The debugsource package, if it exists, that the debuginfo package(s)
+   should Recommend.  */
+Package dbgsrcpkg = findDebugsourcePackage(spec);
 
 #if HAVE_LIBDW
 elf_version (EV_CURRENT);
@@ -3038,6 +3056,12 @@ rpmRC proc

Re: [Rpm-maint] [PATCH] Warn and create empty debugsource package if there are no sources.

[Mark Wielaard]

On Wed, 2017-08-02 at 17:43 +0200, Igor Gnatenko wrote:
> On Wed, 2017-08-02 at 13:09 +0200, Mark Wielaard wrote:
> > Do we want to be nicer to packages that don't include sources like
> > this?
> > Or should we just declare that this is probably a packaging issue
> > (badly produced debuginfo) and improve the error message?
>
> We do this today with debuginfo packages, I don't think debugsource
> should be treated differently. If there will be use-case, then we can
> figure something out.

What precisely does "this" refer to? It currently is treated
differently, that is what the patch tries to fix. Although I am
certainly open to alternatives.

If there are no sources, currently the debuginfo package is just created
without any sources, just the .debug files. But when
_debugsource_packages is defined we error out (with a somewhat cryptic
error message IMHO). It would be nice to make this behavior consistent.
What option do you suggest we should implement?

> > Alternatively we might just skip creation of the whole debugsource
> > package. But I couldn't figure out how to do that.

Cheers,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [PATCH 2/2] Test split debuginfo packages with RemovePathPostfixes.

[Mark Wielaard]

On Fri, 2017-07-28 at 23:24 +0200, Mark Wielaard wrote:
> If split debuginfo code doesn't know about RemovePathPostfixes it fails:
> 
> error: Installed (but unpackaged) file(s) found:
>/usr/lib/debug/bin/hello.foobar-1.0-1.x86_64.debug

This one also now passes with Igor's fixes.

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [PATCH 1/2] Test split debuginfo packages with excluded files.

[Mark Wielaard]

On Fri, 2017-07-28 at 23:24 +0200, Mark Wielaard wrote:
> If split debuginfo code doesn't know about excluded files it fails:
> 
> error: Installed (but unpackaged) file(s) found:
>/usr/lib/debug/bin/hello3-1.0-1.x86_64.debug

And with Igor's fixes this test now passes.
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [PATCH] Add rpmbuild debuginfo subpackages tests.

[Mark Wielaard]

On Fri, 2017-07-28 at 21:55 +0200, Mark Wielaard wrote:
> This adds various tests for making sure multiple subpackages are build
> correctly. Without debuginfo subpackages, with subpackages, subpackages
> with unique debug file and source dir paths and with separate debugsources.

Ping. These are simply tests for existing functionality that didn't have
tests before.
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [PATCH] Warn and create empty debugsource package if there are no sources.

[Mark Wielaard]

Hi,

On Fri, 2017-07-28 at 15:29 +0200, Mark Wielaard wrote:
> +  if [ ! -s "$srcout" ]; then
> +echo >&2 "*** WARNING: No source files found.  Creating empty 
> debugsource package"
> +# Create the empty directory.
> +# See also debugedit invocation. Directories must match up.
> +debug_base_name="$RPM_BUILD_DIR"
> +debug_dest_name="/usr/src/debug"
> +if [ ! -z "$unique_debug_src_base" ]; then
> +  debug_base_name="$BUILDDIR"
> +  debug_dest_name="/usr/src/debug/${unique_debug_src_base}"
> +fi
> +mkdir -p "${RPM_BUILD_ROOT}${debug_dest_name}"
> +echo "$debug_dest_name" > "$srcout"
> +  fi
>  fi

Do we want to be nicer to packages that don't include sources like this?
Or should we just declare that this is probably a packaging issue (badly
produced debuginfo) and improve the error message?

Alternatively we might just skip creation of the whole debugsource
package. But I couldn't figure out how to do that.

Cheers,

Mark

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 1/2] Test split debuginfo packages with excluded files.

[Mark Wielaard]

If split debuginfo code doesn't know about excluded files it fails:

error: Installed (but unpackaged) file(s) found:
   /usr/lib/debug/bin/hello3-1.0-1.x86_64.debug

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 tests/Makefile.am  |  1 +
 tests/data/SPECS/test-subpackages-exclude.spec | 42 
 tests/rpmbuild.at  | 66 ++
 3 files changed, 109 insertions(+)
 create mode 100644 tests/data/SPECS/test-subpackages-exclude.spec

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 7b69ba0..687992f 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -65,6 +65,7 @@ EXTRA_DIST += data/SPECS/prefixtest.spec
 EXTRA_DIST += data/SPECS/testdoc.spec
 EXTRA_DIST += data/SPECS/sigpipe.spec
 EXTRA_DIST += data/SPECS/test-subpackages.spec
+EXTRA_DIST += data/SPECS/test-subpackages-exclude.spec
 EXTRA_DIST += data/SOURCES/hello-1.0-modernize.patch
 EXTRA_DIST += data/SOURCES/hello-1.0.tar.gz
 EXTRA_DIST += data/SOURCES/hello-2.0.tar.gz
diff --git a/tests/data/SPECS/test-subpackages-exclude.spec 
b/tests/data/SPECS/test-subpackages-exclude.spec
new file mode 100644
index 000..e93b10b
--- /dev/null
+++ b/tests/data/SPECS/test-subpackages-exclude.spec
@@ -0,0 +1,42 @@
+Name:   test
+Version:1.0
+Release:1
+Summary:Test
+
+License:Public Domain
+URL:https://fedoraproject.org
+Source: hello.c
+
+%description
+%{summary}.
+
+%package test2
+Summary: Test2.
+%description test2
+
+%prep
+%autosetup -c -D -T
+cp -a %{S:0} .
+
+%build
+gcc -g hello.c -o hello
+cp hello.c hello2.c
+gcc -g hello2.c -o hello2
+cp hello.c hello3.c
+gcc -g hello3.c -o hello3
+
+%install
+mkdir -p %{buildroot}/bin
+install -D -p -m 0755 -t %{buildroot}/bin hello
+install -D -p -m 0755 -t %{buildroot}/bin hello2
+# Install it, but then exclude it...
+install -D -p -m 0755 -t %{buildroot}/bin hello3
+
+%files
+/bin/hello
+
+%files test2
+/bin/hello2
+%exclude /bin/hello3
+
+%changelog
diff --git a/tests/rpmbuild.at b/tests/rpmbuild.at
index e2008da..cbe653d 100644
--- a/tests/rpmbuild.at
+++ b/tests/rpmbuild.at
@@ -1166,3 +1166,69 @@ hello3 debug exists
 ],
 [ignore])
 AT_CLEANUP
+
+# --
+# Check that defining _debuginfo_subpackages works with excluded files.
+AT_SETUP([rpmbuild debuginfo subpackages multiple excluded])
+AT_KEYWORDS([build] [debuginfo] [debugsubpackage] [debugsource])
+AT_CHECK([
+rm -rf ${TOPDIR}
+AS_MKDIR_P(${TOPDIR}/SOURCES)
+
+# Setup sources
+cp "${abs_srcdir}"/data/SOURCES/hello.c ${TOPDIR}/SOURCES
+
+run rpmbuild --quiet \
+  
--macros=${abs_top_builddir}/macros:${abs_top_builddir}/tests/testing/usr/local/lib/rpm/platform/%{_target_cpu}-%{_target_os}/macros:${top_srcdir}/macros.debug
 \
+  --rcfile=${abs_top_builddir}/rpmrc \
+  --define "_unique_debug_names 1" \
+  --define "_unique_debug_srcs 1" \
+  --define "_debugsource_packages 1" \
+  --define "_debuginfo_subpackages 1" \
+  -ba "${abs_srcdir}"/data/SPECS/test-subpackages-exclude.spec
+
+# Check that there are 2 debuginfo packages.
+ls ${abs_builddir}/testing/build/RPMS/*/*debuginfo*rpm | wc --lines
+
+# First contains hello.debug
+rpm2cpio ${abs_builddir}/testing/build/RPMS/*/test-1.0-1.*.rpm \
+  | cpio -diu --quiet
+# Extract the debug name from the exe (.gnu_debuglink section, first string)
+debug_name=$(readelf -p .gnu_debuglink ./bin/hello | grep hello | cut -c13-)
+
+rpm2cpio ${abs_builddir}/testing/build/RPMS/*/test-debuginfo-1.0-1.*.rpm \
+  | cpio -diu --quiet
+if test -f ./usr/lib/debug/bin/$debug_name; then
+  echo "hello debug exists"
+else
+  echo "No hello: $debug_name"
+fi
+
+# Second contains hello2.debug but NOT hello3.debug
+rpm2cpio ${abs_builddir}/testing/build/RPMS/*/test-test2-1.0-1.*.rpm \
+  | cpio -diu --quiet
+# Extract the debug name from the exe (.gnu_debuglink section, first string)
+debug_name=$(readelf -p .gnu_debuglink ./bin/hello2 | grep hello | cut -c13-)
+
+rpm2cpio ${abs_builddir}/testing/build/RPMS/*/test-test2-debuginfo-1.0-1.*.rpm 
\
+  | cpio -diu --quiet
+if test -f ./usr/lib/debug/bin/$debug_name; then
+  echo "hello2 debug exists"
+else
+  echo "No hello2: $debug_name"
+fi
+
+if test -f ./usr/lib/debug/bin/hello3*; then
+  echo "hello3 debug exists"
+else
+  echo "No hello3 debug"
+fi
+],
+[0],
+[2
+hello debug exists
+hello2 debug exists
+No hello3 debug
+],
+[ignore])
+AT_CLEANUP
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 2/2] Test split debuginfo packages with RemovePathPostfixes.

[Mark Wielaard]

If split debuginfo code doesn't know about RemovePathPostfixes it fails:

error: Installed (but unpackaged) file(s) found:
   /usr/lib/debug/bin/hello.foobar-1.0-1.x86_64.debug

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 tests/Makefile.am  |  1 +
 .../data/SPECS/test-subpackages-pathpostfixes.spec | 39 
 tests/rpmbuild.at  | 71 ++
 3 files changed, 111 insertions(+)
 create mode 100644 tests/data/SPECS/test-subpackages-pathpostfixes.spec

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 687992f..03cdea9 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -66,6 +66,7 @@ EXTRA_DIST += data/SPECS/testdoc.spec
 EXTRA_DIST += data/SPECS/sigpipe.spec
 EXTRA_DIST += data/SPECS/test-subpackages.spec
 EXTRA_DIST += data/SPECS/test-subpackages-exclude.spec
+EXTRA_DIST += data/SPECS/test-subpackages-pathpostfixes.spec
 EXTRA_DIST += data/SOURCES/hello-1.0-modernize.patch
 EXTRA_DIST += data/SOURCES/hello-1.0.tar.gz
 EXTRA_DIST += data/SOURCES/hello-2.0.tar.gz
diff --git a/tests/data/SPECS/test-subpackages-pathpostfixes.spec 
b/tests/data/SPECS/test-subpackages-pathpostfixes.spec
new file mode 100644
index 000..19b3b9e
--- /dev/null
+++ b/tests/data/SPECS/test-subpackages-pathpostfixes.spec
@@ -0,0 +1,39 @@
+Name:   test
+Version:1.0
+Release:1
+Summary:Test
+
+License:Public Domain
+URL:https://fedoraproject.org
+Source: hello.c
+
+%description
+%{summary}.
+
+%package test2
+RemovePathPostfixes: .foobar
+Summary: Test2.
+%description test2
+
+%prep
+%autosetup -c -D -T
+cp -a %{S:0} .
+
+%build
+gcc -g hello.c -o hello
+cp hello.c hello2.c
+gcc -g hello2.c -o hello.foobar
+
+%install
+mkdir -p %{buildroot}/bin
+install -D -p -m 0755 -t %{buildroot}/bin hello
+# Install as hello.foobar, but we want the postfix removed in the package...
+install -D -p -m 0755 -t %{buildroot}/bin hello.foobar
+
+%files
+/bin/hello
+
+%files test2
+/bin/hello.foobar
+
+%changelog
diff --git a/tests/rpmbuild.at b/tests/rpmbuild.at
index cbe653d..7864477 100644
--- a/tests/rpmbuild.at
+++ b/tests/rpmbuild.at
@@ -1232,3 +1232,74 @@ No hello3 debug
 ],
 [ignore])
 AT_CLEANUP
+
+# --
+# Check that defining _debuginfo_subpackages works with RemovePathPostfixes.
+AT_SETUP([rpmbuild debuginfo subpackages multiple excluded])
+AT_KEYWORDS([build] [debuginfo] [debugsubpackage] [debugsource])
+AT_CHECK([
+rm -rf ${TOPDIR}
+AS_MKDIR_P(${TOPDIR}/SOURCES)
+
+# Setup sources
+cp "${abs_srcdir}"/data/SOURCES/hello.c ${TOPDIR}/SOURCES
+
+run rpmbuild --quiet \
+  
--macros=${abs_top_builddir}/macros:${abs_top_builddir}/tests/testing/usr/local/lib/rpm/platform/%{_target_cpu}-%{_target_os}/macros:${top_srcdir}/macros.debug
 \
+  --rcfile=${abs_top_builddir}/rpmrc \
+  --define "_unique_debug_names 1" \
+  --define "_unique_debug_srcs 1" \
+  --define "_debugsource_packages 1" \
+  --define "_debuginfo_subpackages 1" \
+  -ba "${abs_srcdir}"/data/SPECS/test-subpackages-pathpostfixes.spec
+
+# Check that there are 2 debuginfo packages.
+ls ${abs_builddir}/testing/build/RPMS/*/*debuginfo*rpm | wc --lines
+
+# First contains hello.debug
+rpm2cpio ${abs_builddir}/testing/build/RPMS/*/test-1.0-1.*.rpm \
+  | cpio -diu --quiet
+# Extract the debug name from the exe (.gnu_debuglink section, first string)
+debug_name=$(readelf -p .gnu_debuglink ./bin/hello | grep hello | cut -c13-)
+
+rpm2cpio ${abs_builddir}/testing/build/RPMS/*/test-debuginfo-1.0-1.*.rpm \
+  | cpio -diu --quiet
+if test -f ./usr/lib/debug/bin/$debug_name; then
+  echo "hello debug exists"
+else
+  echo "No hello: $debug_name"
+fi
+
+# remove it, we are going to check the other debuginfo package.
+rm ./bin/hello
+rm ./usr/lib/debug/bin/$debug_name
+orig_debugname=$debugname
+
+# Second contains hello.foobar.debug but NOT hello.debug
+rpm2cpio ${abs_builddir}/testing/build/RPMS/*/test-test2-1.0-1.*.rpm \
+  | cpio -diu --quiet
+# Extract the debug name from the exe (.gnu_debuglink section, first string)
+debug_name=$(readelf -p .gnu_debuglink ./bin/hello | grep hello | cut -c13-)
+
+rpm2cpio ${abs_builddir}/testing/build/RPMS/*/test-test2-debuginfo-1.0-1.*.rpm 
\
+  | cpio -diu --quiet
+if test -f ./usr/lib/debug/bin/$debug_name; then
+  echo "hello.foobar debug exists"
+else
+  echo "No hello2: $debug_name"
+fi
+
+if test -f ./usr/lib/debug/bin/$orig_debugname; then
+  echo "$orig_debugname exists"
+else
+  echo "No hello.debug"
+fi
+],
+[0],
+[2
+hello debug exists
+hello.foobar debug exists
+No hello.debug
+],
+[ignore])
+AT_CLEANUP
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] Add rpmbuild debuginfo subpackages tests.

[Mark Wielaard]

This adds various tests for making sure multiple subpackages are build
correctly. Without debuginfo subpackages, with subpackages, subpackages
with unique debug file and source dir paths and with separate debugsources.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 tests/Makefile.am  |   1 +
 tests/data/SPECS/test-subpackages.spec |  47 ++
 tests/rpmbuild.at  | 266 +
 3 files changed, 314 insertions(+)
 create mode 100644 tests/data/SPECS/test-subpackages.spec

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 09239ba..7b69ba0 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -64,6 +64,7 @@ EXTRA_DIST += data/SPECS/filetriggers.spec
 EXTRA_DIST += data/SPECS/prefixtest.spec
 EXTRA_DIST += data/SPECS/testdoc.spec
 EXTRA_DIST += data/SPECS/sigpipe.spec
+EXTRA_DIST += data/SPECS/test-subpackages.spec
 EXTRA_DIST += data/SOURCES/hello-1.0-modernize.patch
 EXTRA_DIST += data/SOURCES/hello-1.0.tar.gz
 EXTRA_DIST += data/SOURCES/hello-2.0.tar.gz
diff --git a/tests/data/SPECS/test-subpackages.spec 
b/tests/data/SPECS/test-subpackages.spec
new file mode 100644
index 000..8306600
--- /dev/null
+++ b/tests/data/SPECS/test-subpackages.spec
@@ -0,0 +1,47 @@
+Name:   test
+Version:1.0
+Release:1
+Summary:Test
+
+License:Public Domain
+URL:https://fedoraproject.org
+Source: hello.c
+
+%description
+%{summary}.
+
+%package test2
+Summary: Test2.
+%description test2
+
+%package test3
+Summary: Test3.
+%description test3
+
+%prep
+%autosetup -c -D -T
+cp -a %{S:0} .
+
+%build
+gcc -g hello.c -o hello
+cp hello.c hello2.c
+gcc -g hello2.c -o hello2
+cp hello.c hello3.c
+gcc -g hello3.c -o hello3
+
+%install
+mkdir -p %{buildroot}/bin
+install -D -p -m 0755 -t %{buildroot}/bin hello
+install -D -p -m 0755 -t %{buildroot}/bin hello2
+install -D -p -m 0755 -t %{buildroot}/bin hello3
+
+%files
+/bin/hello
+
+%files test2
+/bin/hello2
+
+%files test3
+/bin/hello3
+
+%changelog
diff --git a/tests/rpmbuild.at b/tests/rpmbuild.at
index 3f41652..e2008da 100644
--- a/tests/rpmbuild.at
+++ b/tests/rpmbuild.at
@@ -900,3 +900,269 @@ ls ./usr/src/debug/test-1.0*/
 ],
 [ignore])
 AT_CLEANUP
+
+# --
+# Check that undefining _debuginfo_subpackages creates one single -debuginfo.
+AT_SETUP([rpmbuild debuginfo subpackages single])
+AT_KEYWORDS([build] [debuginfo] [debugsubpackage])
+AT_CHECK([
+rm -rf ${TOPDIR}
+AS_MKDIR_P(${TOPDIR}/SOURCES)
+
+# Setup sources
+cp "${abs_srcdir}"/data/SOURCES/hello.c ${TOPDIR}/SOURCES
+
+run rpmbuild --quiet \
+  
--macros=${abs_top_builddir}/macros:${abs_top_builddir}/tests/testing/usr/local/lib/rpm/platform/%{_target_cpu}-%{_target_os}/macros:${top_srcdir}/macros.debug
 \
+  --rcfile=${abs_top_builddir}/rpmrc \
+  --undefine "_unique_debug_names" \
+  --undefine "_unique_debug_srcs" \
+  --undefine "_debugsource_packages" \
+  --undefine "_debuginfo_subpackages" \
+  -ba "${abs_srcdir}"/data/SPECS/test-subpackages.spec
+
+# Check that there are is just one debuginfo packages.
+ls ${abs_builddir}/testing/build/RPMS/*/*debuginfo*rpm | wc --lines
+
+# Which contains hello.debug
+rpm2cpio ${abs_builddir}/testing/build/RPMS/*/test-1.0-1.*.rpm \
+  | cpio -diu --quiet
+# Extract the debug name from the exe (.gnu_debuglink section, first string)
+debug_name=$(readelf -p .gnu_debuglink ./bin/hello | grep hello | cut -c13-)
+
+rpm2cpio ${abs_builddir}/testing/build/RPMS/*/test-debuginfo-1.0-1.*.rpm \
+  | cpio -diu --quiet
+if test -f ./usr/lib/debug/bin/$debug_name; then
+  echo "hello debug exists"
+else
+  echo "No hello: $debug_name"
+fi
+],
+[0],
+[1
+hello debug exists
+],
+[ignore])
+AT_CLEANUP
+
+# --
+# Check that defining _debuginfo_subpackages creates multiple -debuginfos.
+AT_SETUP([rpmbuild debuginfo subpackages multiple])
+AT_KEYWORDS([build] [debuginfo] [debugsubpackage])
+AT_CHECK([
+rm -rf ${TOPDIR}
+AS_MKDIR_P(${TOPDIR}/SOURCES)
+
+# Setup sources
+cp "${abs_srcdir}"/data/SOURCES/hello.c ${TOPDIR}/SOURCES
+
+run rpmbuild --quiet \
+  
--macros=${abs_top_builddir}/macros:${abs_top_builddir}/tests/testing/usr/local/lib/rpm/platform/%{_target_cpu}-%{_target_os}/macros:${top_srcdir}/macros.debug
 \
+  --rcfile=${abs_top_builddir}/rpmrc \
+  --undefine "_unique_debug_names" \
+  --undefine "_unique_debug_srcs" \
+  --undefine "_debugsource_packages" \
+  --define "_debuginfo_subpackages 1" \
+  -ba "${abs_srcdir}"/data/SPECS/test-subpackages.spec
+
+# Check that there are 3 debuginfo packages.
+ls ${abs_builddir}/testing/build/RPMS/*/*debuginfo*rpm | wc --lines
+
+# First contains hello.debug
+rpm2cpio ${abs_builddir}/testing/build/RPMS/*/test-1.0-1.*.rpm \
+  | cpio -diu --quiet
+# Extract the debug name from the exe (.gnu_debuglink section,

[Rpm-maint] [PATCH] Warn and create empty debugsource package if there are no sources.

[Mark Wielaard]

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 scripts/find-debuginfo.sh | 13 +
 tests/rpmbuild.at | 33 +
 2 files changed, 46 insertions(+)

diff --git a/scripts/find-debuginfo.sh b/scripts/find-debuginfo.sh
index d8725ad..9b3815a 100755
--- a/scripts/find-debuginfo.sh
+++ b/scripts/find-debuginfo.sh
@@ -573,6 +573,19 @@ if [ -n "$srcout" ]; then
  find src/debug -mindepth 1 -maxdepth 1
 ) | sed 's,^,/usr/,' >> "$srcout"
   fi
+  if [ ! -s "$srcout" ]; then
+echo >&2 "*** WARNING: No source files found.  Creating empty debugsource 
package"
+# Create the empty directory.
+# See also debugedit invocation. Directories must match up.
+debug_base_name="$RPM_BUILD_DIR"
+debug_dest_name="/usr/src/debug"
+if [ ! -z "$unique_debug_src_base" ]; then
+  debug_base_name="$BUILDDIR"
+  debug_dest_name="/usr/src/debug/${unique_debug_src_base}"
+fi
+mkdir -p "${RPM_BUILD_ROOT}${debug_dest_name}"
+echo "$debug_dest_name" > "$srcout"
+  fi
 fi
 
 # Append to $1 only the lines from stdin not already in the file.
diff --git a/tests/rpmbuild.at b/tests/rpmbuild.at
index 3f41652..2359c63 100644
--- a/tests/rpmbuild.at
+++ b/tests/rpmbuild.at
@@ -900,3 +900,36 @@ ls ./usr/src/debug/test-1.0*/
 ],
 [ignore])
 AT_CLEANUP
+
+# --
+# Check that defining _debugsource_packages creates -debugsource package
+# doesn't break when there are no source files (found).
+AT_SETUP([rpmbuild debugsource no source files])
+AT_KEYWORDS([build] [debuginfo] [debugsource])
+AT_CHECK([
+rm -rf ${TOPDIR}
+AS_MKDIR_P(${TOPDIR}/SOURCES)
+
+# Setup sources
+cp "${abs_srcdir}"/data/SOURCES/hello-1.0.tar.gz 
"${abs_srcdir}"/data/SOURCES/hello-1.0-modernize.patch ${TOPDIR}/SOURCES
+
+# Build a package that doesn't use -g.
+run rpmbuild --quiet \
+  
--macros=${abs_top_builddir}/macros:${abs_top_builddir}/tests/testing/usr/local/lib/rpm/platform/%{_target_cpu}-%{_target_os}/macros:${top_srcdir}/macros.debug
 \
+  --rcfile=${abs_top_builddir}/rpmrc \
+  --define "_debugsource_packages 1" \
+  -ba "${abs_srcdir}"/data/SPECS/hello.spec
+
+# Unpack the debugsource rpm so we can check the sources are (not) there.
+rpm2cpio ${abs_builddir}/testing/build/RPMS/*/hello-debugsource-1.0-1.*.rpm \
+  | cpio -diu --quiet
+
+# There should be just one empty directory (strip off nvra).
+find ./usr/src/debug/* | cut -f1 -d-
+
+],
+[0],
+[./usr/src/debug/hello
+],
+[ignore])
+AT_CLEANUP
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] Add two testcases for generating debugsource packages.

[Mark Wielaard]

These tests define _debugsource_packages and check a debugsource package
is created that contains the source of the program created.

Without the fix to make sure that debugsourcefiles.list is generated in
the build dir the second testcase will fail with:
error: Could not open %files file

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 tests/Makefile.am  |  1 +
 tests/data/SPECS/hello-cd.spec | 29 
 tests/rpmbuild.at  | 62 ++
 3 files changed, 92 insertions(+)
 create mode 100644 tests/data/SPECS/hello-cd.spec

diff --git a/tests/Makefile.am b/tests/Makefile.am
index b962e4a..09239ba 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -78,6 +78,7 @@ EXTRA_DIST += data/SRPMS/hello-1.0-1.src.rpm
 EXTRA_DIST += data/SOURCES/hello.c
 EXTRA_DIST += data/SPECS/hello-attr-buildid.spec
 EXTRA_DIST += data/SPECS/hello-config-buildid.spec
+EXTRA_DIST += data/SPECS/hello-cd.spec
 EXTRA_DIST += data/keys/rpm.org-rsa-2048-test.pub
 EXTRA_DIST += data/keys/rpm.org-rsa-2048-test.secret
 
diff --git a/tests/data/SPECS/hello-cd.spec b/tests/data/SPECS/hello-cd.spec
new file mode 100644
index 000..ab1e300
--- /dev/null
+++ b/tests/data/SPECS/hello-cd.spec
@@ -0,0 +1,29 @@
+Name:   test
+Version:1.0
+Release:1
+Summary:Test
+
+License:Public Domain
+URL:https://fedoraproject.org
+Source: hello.c
+
+%description
+%{summary}.
+
+%prep
+%autosetup -c -D -T
+cp -a %{S:0} .
+
+%build
+gcc -g hello.c -o hello
+
+%install
+mkdir -p %{buildroot}/bin
+install -D -p -m 0755 -t %{buildroot}/bin hello
+# Pretend we need to do something in /tmp
+cd /tmp
+
+%files
+%attr(644,root,root) /bin/hello
+
+%changelog
diff --git a/tests/rpmbuild.at b/tests/rpmbuild.at
index 3200fb2..3f41652 100644
--- a/tests/rpmbuild.at
+++ b/tests/rpmbuild.at
@@ -838,3 +838,65 @@ hello-1.0
 ],
 [ignore])
 AT_CLEANUP
+
+# --
+# Check that defining _debugsource_packages creates -debugsource package
+AT_SETUP([rpmbuild debugsource])
+AT_KEYWORDS([build] [debuginfo] [debugsource])
+AT_CHECK([
+rm -rf ${TOPDIR}
+AS_MKDIR_P(${TOPDIR}/SOURCES)
+
+# Build a package that has some debuginfo
+cp "${abs_srcdir}"/data/SOURCES/hello-1.0.tar.gz 
"${abs_srcdir}"/data/SOURCES/hello-1.0-modernize.patch ${TOPDIR}/SOURCES
+
+run rpmbuild --quiet \
+  
--macros=${abs_top_builddir}/macros:${abs_top_builddir}/tests/testing/usr/local/lib/rpm/platform/%{_target_cpu}-%{_target_os}/macros:${top_srcdir}/macros.debug
 \
+  --rcfile=${abs_top_builddir}/rpmrc \
+  --define "_debugsource_packages 1" \
+  -ba "${abs_srcdir}"/data/SPECS/hello2.spec
+
+# Unpack the debuginfo rpms so we can check the sources are there.
+rpm2cpio ${abs_builddir}/testing/build/RPMS/*/hello2-debugsource-1.0-1.*.rpm \
+  | cpio -diu --quiet
+
+# Check that hello.c is there.
+ls ./usr/src/debug/hello2-1.0*/
+],
+[0],
+[hello.c
+],
+[ignore])
+AT_CLEANUP
+
+# --
+# Check that defining _debugsource_packages creates -debugsource package
+# even if %install changes the working directory (debugsourcefiles.list
+# should be in expected build dir).
+AT_SETUP([rpmbuild debugsource debugsourcefiles.list path])
+AT_KEYWORDS([build] [debuginfo] [debugsource])
+AT_CHECK([
+rm -rf ${TOPDIR}
+AS_MKDIR_P(${TOPDIR}/SOURCES)
+
+# Setup sources
+cp "${abs_srcdir}"/data/SOURCES/hello.c ${TOPDIR}/SOURCES
+
+run rpmbuild --quiet \
+  
--macros=${abs_top_builddir}/macros:${abs_top_builddir}/tests/testing/usr/local/lib/rpm/platform/%{_target_cpu}-%{_target_os}/macros:${top_srcdir}/macros.debug
 \
+  --rcfile=${abs_top_builddir}/rpmrc \
+  --define "_debugsource_packages 1" \
+  -ba "${abs_srcdir}"/data/SPECS/hello-cd.spec
+
+# Unpack the debuginfo rpms so we can check the sources are there.
+rpm2cpio ${abs_builddir}/testing/build/RPMS/*/test-debugsource-1.0-1.*.rpm \
+  | cpio -diu --quiet
+
+# Check that hello.c is there.
+ls ./usr/src/debug/test-1.0*/
+],
+[0],
+[hello.c
+],
+[ignore])
+AT_CLEANUP
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] Make sure that debugsourcefiles.list is generated in the build dir.

[Mark Wielaard]

The %_debugsource_template expects the debugsourcefiles.list file
to be in the (current) build dir. Make sure that is always the case
even if find-debuginfo.sh was invoked in a different (sub) directory
by adding the build dir path as explicit argument to -S.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 macros.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/macros.in b/macros.in
index 8518228..147db83 100644
--- a/macros.in
+++ b/macros.in
@@ -185,7 +185,7 @@
 %{?_unique_debug_srcs:--unique-debug-src-base 
"%{name}-%{VERSION}-%{RELEASE}.%{_arch}"} \\\
 %{?_find_debuginfo_dwz_opts} \\\
 %{?_find_debuginfo_opts} \\\
-%{?_debugsource_packages:-S debugsourcefiles.list} \\\
+%{?_debugsource_packages:-S 
"%{_builddir}/%{?buildsubdir}/debugsourcefiles.list"} \\\
 "%{_builddir}/%{?buildsubdir}"\
 %{nil}
 
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [PATCH] config: Detect major/minor warnings and include the correct system header.

[Mark Wielaard]

Hi,

On Thu, 2017-07-20 at 17:06 +0300, Dmitry V. Levin wrote:
> I'm not sure what sys/mkdev.h does, but glibc's sys/sysmacros.h certainly
> undefines major, minor, and makedev prior to defining its own versions of
> these macros.
> 
> My guess is that these undefs are not needed.

You are right. This makes the patch simpler. Now we only need to fix the
AC_HEADER_MAJOR check. Cleaned up patch attached. Retested on rhel7 and
fedora26.

sys/mkdev.h is a solaris/bsd thing. I checked and they also undef the
macros before (re)defining them. Although there they might not be
defined in another header in the first place.

Thanks,

Mark
From 77a745abd88f278b4c29aadefb2f6c5d47bde5e6 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <m...@klomp.org>
Date: Wed, 19 Jul 2017 14:43:28 +0200
Subject: [PATCH] config: Detect major/minor warnings and include the correct
 system header.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

glibc 2.25 introduced (really long and annoying) warnings for each use
of the major/minor macros from the wrong header:

lib/cpio.c: In function ‘rpmcpioHeaderWrite’:
lib/cpio.c:245:13: warning: In the GNU C Library, "major" is defined
 by . For historical compatibility, it is
 currently defined by  as well, but we plan to
 remove this soon. To use "major", include 
 directly. If you did not intend to use a system-defined macro
 "major", you should undefine it after including .
 dev = major(st->st_dev); SET_NUM_FIELD(hdr->devMajor, dev, field);
 ^~

Adjust the configure check to correctly detect the header to include
that doesn't produce those warning producing macros.

Tested against RHEL7 (glibc 2.17) and Fedora 26 (glibc 2.25).

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 configure.ac | 8 
 1 file changed, 8 insertions(+)

diff --git a/configure.ac b/configure.ac
index cc657ec..017a908 100644
--- a/configure.ac
+++ b/configure.ac
@@ -561,7 +561,15 @@ AM_ICONV
 
 dnl Checks for header files we can live without.
 AC_HEADER_STDC
+dnl glibc and autoconf don't really play well together.
+dnl glibc will produce a warning when including the wrong header.
+dnl but still define major and minor. Causing us to include the header
+dnl that produces a giant warning for each major/minor use.
+dnl Use -Werror to work around that.
+old_CFLAGS=$CFLAGS
+CFLAGS="$CFLAGS -Werror"
 AC_HEADER_MAJOR
+CFLAGS=$old_CFLAGS
 AC_STRUCT_DIRENT_D_TYPE
 
 AC_CHECK_HEADERS(limits.h)
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] replaceSigDigests is only used with IMAEVM.

[Mark Wielaard]

The replaceSigDigests function is only used in includeFileSignatures
when WITH_IMAEVM is defined. If not warning is generated.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 sign/rpmgensig.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sign/rpmgensig.c b/sign/rpmgensig.c
index 0731363..d29c178 100644
--- a/sign/rpmgensig.c
+++ b/sign/rpmgensig.c
@@ -469,6 +469,7 @@ static void unloadImmutableRegion(Header *hdrp, rpmTagVal 
tag)
 }
 }
 
+#ifdef WITH_IMAEVM
 static rpmRC replaceSigDigests(FD_t fd, const char *rpm, Header *sigp,
   off_t sigStart, off_t sigTargetSize,
   char *SHA256, char *SHA1, uint8_t *MD5)
@@ -516,6 +517,7 @@ static rpmRC replaceSigDigests(FD_t fd, const char *rpm, 
Header *sigp,
 exit:
 return rc;
 }
+#endif
 
 static rpmRC includeFileSignatures(FD_t fd, const char *rpm,
   Header *sigp, Header *hdrp,
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] find-debuginfo.sh: Remove non-allocated NOBITS sections from minisymtab.

[Mark Wielaard]

In the minisymtab section (the .gnu_debugdata embedded ELF image) we
do not need unallocated sections (except for the SYMTAB and STRTAB
sections we are creating). We already remove PROGBITS and NOTES. Also
remove NOBITS sections. They should not really take up much (any) space
but they still add to the section tables. These sections might be created
with the new --keep-section support (which puts the actual section in
the main ELF binary, and a NOBITS variant in the .debug file).

Also binutils objcopy seems to sometimes add them anyway filled with
zeros instead of marking them NOBITS.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 scripts/find-debuginfo.sh | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/scripts/find-debuginfo.sh b/scripts/find-debuginfo.sh
index e19ce9b..2fa95e8 100755
--- a/scripts/find-debuginfo.sh
+++ b/scripts/find-debuginfo.sh
@@ -247,12 +247,16 @@ add_minidebug()
   local mini_debuginfo=`mktemp`
 
   # In the minisymtab we don't need the .debug_ sections (already removed
-  # by -S) but also not any other non-allocated PROGBITS or NOTE sections.
+  # by -S) but also not other non-allocated PROGBITS, NOTE or NOBITS sections.
   # List and remove them explicitly. We do want to keep the allocated,
   # symbol and NOBITS sections so cannot use --keep-only because that is
   # too aggressive. Field $2 is the section name, $3 is the section type
   # and $8 are the section flags.
-  local remove_sections=`readelf -W -S "$debuginfo" | awk '{ if 
(index($2,".debug_") != 1 && ($3 == "PROGBITS" || $3 == "NOTE") && 
index($8,"A") == 0) printf "--remove-section "$2" " }'`
+  local remove_sections=`readelf -W -S "$debuginfo" \
+   | awk '{ if (index($2,".debug_") != 1 \
+&& ($3 == "PROGBITS" || $3 == "NOTE" || $3 == "NOBITS") \
+&& index($8,"A") == 0) \
+  printf "--remove-section "$2" " }'`
 
   # Extract the dynamic symbols from the main binary, there is no need to also 
have these
   # in the normal symbol table
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] config: Detect major/minor warnings and include the correct system header.

[Mark Wielaard]

glibc 2.25 introduced (really long and annoying) warnings for each use
of the major/minor macros from the wrong header:

lib/cpio.c: In function ‘rpmcpioHeaderWrite’:
lib/cpio.c:245:13: warning: In the GNU C Library, "major" is defined
 by . For historical compatibility, it is
 currently defined by  as well, but we plan to
 remove this soon. To use "major", include 
 directly. If you did not intend to use a system-defined macro
 "major", you should undefine it after including .
 dev = major(st->st_dev); SET_NUM_FIELD(hdr->devMajor, dev, field);
 ^~

Adjust the configure check and undef the warning producing macros to get
rid of the wrong definitions and use the macros from the right include.

Tested against RHEL7 (glibc 2.17) and Fedora 26 (glibc 2.25).

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 configure.ac | 8 
 lib/cpio.c   | 6 ++
 2 files changed, 14 insertions(+)

diff --git a/configure.ac b/configure.ac
index cc657ec..017a908 100644
--- a/configure.ac
+++ b/configure.ac
@@ -561,7 +561,15 @@ AM_ICONV
 
 dnl Checks for header files we can live without.
 AC_HEADER_STDC
+dnl glibc and autoconf don't really play well together.
+dnl glibc will produce a warning when including the wrong header.
+dnl but still define major and minor. Causing us to include the header
+dnl that produces a giant warning for each major/minor use.
+dnl Use -Werror to work around that.
+old_CFLAGS=$CFLAGS
+CFLAGS="$CFLAGS -Werror"
 AC_HEADER_MAJOR
+CFLAGS=$old_CFLAGS
 AC_STRUCT_DIRENT_D_TYPE
 
 AC_CHECK_HEADERS(limits.h)
diff --git a/lib/cpio.c b/lib/cpio.c
index 57c9592..b7ba27d 100644
--- a/lib/cpio.c
+++ b/lib/cpio.c
@@ -9,9 +9,15 @@
 
 #include "system.h"
 
+/* system.h might already have included the wrong header, undef major
+   and minor and get the real definition from one of the correct headers. */
 #if MAJOR_IN_MKDEV
+#undef major
+#undef minor
 #include 
 #elif MAJOR_IN_SYSMACROS
+#undef major
+#undef minor
 #include 
 #else
 #include  /* already included from system.h */
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] find-debuginfo.sh: Add --keep-section and --remove-section for eu-strip.

[Mark Wielaard]

Use --keep-section SECTION or --remove-section SECTION to explicitly
keep a (non-allocated) section in the main executable or explicitly
remove it into the .debug file. SECTION is an extended wildcard pattern.
Both options can be given more than once.

https://bugzilla.redhat.com/show_bug.cgi?id=1465997

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 scripts/find-debuginfo.sh | 21 +++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/scripts/find-debuginfo.sh b/scripts/find-debuginfo.sh
index 5613391..e19ce9b 100755
--- a/scripts/find-debuginfo.sh
+++ b/scripts/find-debuginfo.sh
@@ -3,6 +3,7 @@
 #for inclusion in an rpm spec file.
 #
 # Usage: find-debuginfo.sh [--strict-build-id] [-g] [-r] [-m] [-i] [-n]
+# [--keep-section SECTION] [--remove-section SECTION]
 # [-j N]
 # [-o debugfiles.list]
 # [-S debugsourcefiles.list]
@@ -15,9 +16,14 @@
 # [builddir]
 #
 # The -g flag says to use strip -g instead of full strip on DSOs or EXEs.
+# The -r flag says to use eu-strip --reloc-debug-sections.
+# Use --keep-section SECTION or --remove-section SECTION to explicitly
+# keep a (non-allocated) section in the main executable or explicitly
+# remove it into the .debug file. SECTION is an extended wildcard pattern.
+# Both options can be given more than once.
+#
 # The --strict-build-id flag says to exit with failure status if
 # any ELF binary processed fails to contain a build-id note.
-# The -r flag says to use eu-strip --reloc-debug-sections.
 # The -m flag says to include a .gnu_debugdata section in the main binary.
 # The -i flag says to include a .gdb_index section in the .debug file.
 # The -n flag says to not recompute the build-id.
@@ -65,6 +71,9 @@ strip_g=false
 # with -r arg, pass --reloc-debug-sections to eu-strip.
 strip_r=false
 
+# keep or remove arguments to eu-strip.
+keep_remove_args=
+
 # with -m arg, add minimal debuginfo to binary.
 include_minidebug=false
 
@@ -158,6 +167,14 @@ while [ $# -gt 0 ]; do
   -r)
 strip_r=true
 ;;
+  --keep-section)
+keep_remove_args="${keep_remove_args} --keep-section $2"
+shift
+;;
+  --remove-section)
+keep_remove_args="${keep_remove_args} --remove-section $2"
+shift
+;;
   -j)
 n_jobs=$2
 shift
@@ -215,7 +232,7 @@ strip_to_debug()
   application/x-sharedlib*) g=-g ;;
   application/x-executable*) g=-g ;;
   esac
-  eu-strip --remove-comment $r $g -f "$1" "$2" || exit
+  eu-strip --remove-comment $r $g ${keep_remove_args} -f "$1" "$2" || exit
   chmod 444 "$1" || exit
 }
 
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 2/3] Use a file list to add build-id files to pkgList.

[Mark Wielaard]

Change the generation of build-id files to a file list using ARGV_t.
First go through the current package list and generate a files list.
Then add those files as if they were part of the original package file
list using the new resetPackageFilesDefaults() and addPackageFileList().

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 build/files.c | 74 ++-
 1 file changed, 33 insertions(+), 41 deletions(-)

diff --git a/build/files.c b/build/files.c
index c7debdc..cb7def6 100644
--- a/build/files.c
+++ b/build/files.c
@@ -1614,6 +1614,15 @@ exit:
 return rc;
 }
 
+/* add a directory to the file list */
+static void argvAddDir(ARGV_t *filesp, const char *dir)
+{
+char *line = NULL;
+rasprintf(, "%%dir %s", dir);
+argvAdd(filesp, line);
+_free(line);
+}
+
 #if HAVE_LIBDW
 /* How build id links are generated.  See macros.in for description.  */
 #define BUILD_IDS_NONE 0
@@ -1621,7 +1630,7 @@ exit:
 #define BUILD_IDS_SEPARATE 2
 #define BUILD_IDS_COMPAT   3
 
-static int addNewIDSymlink(FileList fl,
+static int addNewIDSymlink(ARGV_t *files,
   char *targetpath, char *idlinkpath,
   int isDbg, int *dups)
 {
@@ -1670,8 +1679,7 @@ static int addNewIDSymlink(FileList fl,
rpmlog(RPMLOG_ERR, "%s: %s -> %s: %m\n",
   linkerr, linkpath, targetpath);
 } else {
-   fl->cur.isDir = 0;
-   rc = addFile(fl, linkpath, NULL);
+   rc = argvAdd(files, linkpath);
 }
 
 if (nr > 0) {
@@ -1709,7 +1717,7 @@ static int addNewIDSymlink(FileList fl,
 return rc;
 }
 
-static int generateBuildIDs(FileList fl)
+static int generateBuildIDs(FileList fl, ARGV_t *files)
 {
 int rc = 0;
 int i;
@@ -1858,18 +1866,9 @@ static int generateBuildIDs(FileList fl)
mainiddir = rpmGetPath(fl->buildRoot, BUILD_ID_DIR, NULL);
debugiddir = rpmGetPath(fl->buildRoot, DEBUG_ID_DIR, NULL);
 
-   /* Make sure to reset all file flags to defaults.
-  Uses parseForAttr to reset ar, arFlags, and specdFlags.
-  Note that parseForAttr pokes at the attrstr, so we cannot
-  just pass a static string. */
-   fl->cur.attrFlags = 0;
-   fl->def.attrFlags = 0;
-   fl->def.verifyFlags = RPMVERIFY_ALL;
-   fl->cur.verifyFlags = RPMVERIFY_ALL;
-   fl->def.specdFlags |= SPECD_VERIFY;
-   fl->cur.specdFlags |= SPECD_VERIFY;
+   /* Make sure to reset all file flags to defaults.  */
attrstr = mkattr(NULL);
-   parseForAttr(fl->pool, attrstr, 1, >def);
+   argvAdd(files, attrstr);
free (attrstr);
 
/* Supported, but questionable.  */
@@ -1881,11 +1880,7 @@ static int generateBuildIDs(FileList fl)
if ((rc = rpmioMkpath(mainiddir, 0755, -1, -1)) != 0) {
rpmlog(RPMLOG_ERR, "%s %s: %m\n", errdir, mainiddir);
} else {
-   attrstr = mkattr(mainiddir);
-   parseForAttr(fl->pool, attrstr, 0, >cur);
-   fl->cur.isDir = 1;
-   rc = addFile(fl, mainiddir, NULL);
-   free (attrstr);
+   argvAddDir(files, mainiddir);
}
}
 
@@ -1893,11 +1888,7 @@ static int generateBuildIDs(FileList fl)
if ((rc = rpmioMkpath(debugiddir, 0755, -1, -1)) != 0) {
rpmlog(RPMLOG_ERR, "%s %s: %m\n", errdir, debugiddir);
} else {
-   attrstr = mkattr(debugiddir);
-   parseForAttr(fl->pool, attrstr, 0, >cur);
-   fl->cur.isDir = 1;
-   rc = addFile(fl, debugiddir, NULL);
-   free (attrstr);
+   argvAddDir(files, debugiddir);
}
}
}
@@ -1936,9 +1927,9 @@ static int generateBuildIDs(FileList fl)
&& (rc = rpmioMkpath(buildidsubdir, 0755, -1, -1)) != 0) {
rpmlog(RPMLOG_ERR, "%s %s: %m\n", errdir, buildidsubdir);
} else {
-   fl->cur.isDir = 1;
-   if (!addsubdir
-   || (rc = addFile(fl, buildidsubdir, NULL)) == 0) {
+   if (addsubdir)
+  argvAddDir (files, buildidsubdir);
+   if (rc == 0) {
char *linkpattern, *targetpattern;
char *linkpath, *targetpath;
int dups = 0;
@@ -1952,7 +1943,7 @@ static int generateBuildIDs(FileList fl)
rasprintf(, linkpattern,
  buildidsubdir, [i][2]);
rasprintf(, targetpattern, paths[i]);
-   rc = addNewIDSymlink(fl, targetpath, linkpath,

[Rpm-maint] [PATCH 3/3] Change mkattr to always create a %defattr with explicitly set modes.

[Mark Wielaard]

mkattr used "-" as default mode which would pick up the mode for files
as they were on disk. This could cause files generated by rpmbuild to
use a "non-standard" mode if umask was set by the user. Explitictly
use 755 for directories and 644 for files to make builds independent
of any umask settings.

This works as is for both files and directories, so no file argument
is necessary anymore.

https://bugzilla.redhat.com/show_bug.cgi?id=1452893
https://bugzilla.redhat.com/show_bug.cgi?id=1458839

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 build/files.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/build/files.c b/build/files.c
index cb7def6..dcc772e 100644
--- a/build/files.c
+++ b/build/files.c
@@ -203,13 +203,12 @@ static void dupAttrRec(const AttrRec oar, AttrRec nar)
 *nar = *oar; /* struct assignment */
 }
 
-static char *mkattr(const char *fn)
+/* Creates a default $defattr string. Can be used with argvAdd().
+   Caller owns the new string which needs to be freed when done.  */
+static char *mkattr(void)
 {
 char *s = NULL;
-if (fn)
-   rasprintf(, "%s(-,%s,%s) %s", "%attr", UID_0_USER, GID_0_GROUP, fn);
-else
-   rasprintf(, "%s(-,%s,%s)", "%defattr", UID_0_USER, GID_0_GROUP);
+rasprintf(, "%s(644,%s,%s,755)", "%defattr", UID_0_USER, GID_0_GROUP);
 return s;
 }
 
@@ -1867,7 +1866,7 @@ static int generateBuildIDs(FileList fl, ARGV_t *files)
debugiddir = rpmGetPath(fl->buildRoot, DEBUG_ID_DIR, NULL);
 
/* Make sure to reset all file flags to defaults.  */
-   attrstr = mkattr(NULL);
+   attrstr = mkattr();
argvAdd(files, attrstr);
free (attrstr);
 
@@ -2825,7 +2824,7 @@ static void filterDebuginfoPackage(rpmSpec spec, Package 
pkg,
if (access(path, F_OK) == 0) {
/* Append the file list preamble */
if (!files) {
-   char *attr = mkattr(NULL);
+   char *attr = mkattr();
argvAdd(, attr);
argvAddDir(, DEBUG_LIB_DIR);
free(attr);
@@ -2885,7 +2884,7 @@ static int addDebugDwz(Package pkg, char *buildroot)
 rasprintf(, "%s%s", buildroot, DEBUG_DWZ_DIR);
 if (lstat(path, ) == 0 && S_ISDIR(sbuf.st_mode)) {
if (!pkg->fileList) {
-   char *attr = mkattr(NULL);
+   char *attr = mkattr();
argvAdd(>fileList, attr);
argvAddDir(>fileList, DEBUG_LIB_DIR);
free(attr);
@@ -2919,7 +2918,7 @@ static int addDebugSrc(Package pkg, char *buildroot)
continue;
rasprintf(, "%s/%s", DEBUG_SRC_DIR, de->d_name);
if (!pkg->fileList) {
-   char *attr = mkattr(NULL);
+   char *attr = mkattr();
argvAdd(>fileList, attr);
free(attr);
}
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 1/3] Extract package file list processing in separate functions.

[Mark Wielaard]

Extract two functions resetPackageFilesDefaults() and addPackageFileList()
from processPackageFiles(). This will make it possible to add multiple
(generated) file lists to a package later.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 build/files.c | 170 --
 1 file changed, 107 insertions(+), 63 deletions(-)

diff --git a/build/files.c b/build/files.c
index 6504b8c..c7debdc 100644
--- a/build/files.c
+++ b/build/files.c
@@ -2349,45 +2349,35 @@ static void processSpecialDir(rpmSpec spec, Package 
pkg, FileList fl,
 freeStringBuf(docScript);
 free(mkdocdir);
 }
-   
 
-static rpmRC processPackageFiles(rpmSpec spec, rpmBuildPkgFlags pkgFlags,
-Package pkg, int didInstall, int test)
+
+/* Resets the default settings for files in the package list.
+   Used in processPackageFiles whenever a new set of files is added. */
+static void resetPackageFilesDefaults (struct FileList_s *fl,
+  rpmBuildPkgFlags pkgFlags)
 {
 struct AttrRec_s root_ar = { 0, 0, 0, 0, 0, 0 };
-struct FileList_s fl;
-ARGV_t fileNames = NULL;
-specialDir specialDoc = NULL;
-specialDir specialLic = NULL;
 
-pkg->cpioList = NULL;
+root_ar.ar_user = rpmstrPoolId(fl->pool, UID_0_USER, 1);
+root_ar.ar_group = rpmstrPoolId(fl->pool, GID_0_GROUP, 1);
+dupAttrRec(_ar, >def.ar); /* XXX assume %defattr(-,root,root) */
 
-for (ARGV_const_t fp = pkg->fileFile; fp && *fp != NULL; fp++) {
-   if (readFilesManifest(spec, pkg, *fp))
-   return RPMRC_FAIL;
-}
-/* Init the file list structure */
-memset(, 0, sizeof(fl));
-
-fl.pool = rpmstrPoolLink(spec->pool);
-/* XXX spec->buildRoot == NULL, then xstrdup("") is returned */
-fl.buildRoot = rpmGenPath(spec->rootDir, spec->buildRoot, NULL);
-fl.buildRootLen = strlen(fl.buildRoot);
+fl->def.verifyFlags = RPMVERIFY_ALL;
 
-root_ar.ar_user = rpmstrPoolId(fl.pool, UID_0_USER, 1);
-root_ar.ar_group = rpmstrPoolId(fl.pool, GID_0_GROUP, 1);
-dupAttrRec(_ar, );  /* XXX assume %defattr(-,root,root) */
-
-fl.def.verifyFlags = RPMVERIFY_ALL;
-
-fl.pkgFlags = pkgFlags;
+fl->pkgFlags = pkgFlags;
+}
 
-{  char *docs = rpmGetPath("%{?__docdir_path}", NULL);
-   argvSplit(, docs, ":");
-   free(docs);
-}
-
-for (ARGV_const_t fp = pkg->fileList; *fp != NULL; fp++) {
+/* Adds the given fileList to the package. If fromSpecFileList is not zero
+   then the specialDirs are also filled in and the files are sanitized
+   through processBinaryFile(). Otherwise no special files are processed
+   and the files are added directly through addFile().  */
+static void addPackageFileList (struct FileList_s *fl, Package pkg,
+   ARGV_t *fileList,
+   specialDir *specialDoc, specialDir *specialLic,
+   int fromSpecFileList)
+{
+ARGV_t fileNames = NULL;
+for (ARGV_const_t fp = *fileList; *fp != NULL; fp++) {
char buf[strlen(*fp) + 1];
const char *s = *fp;
SKIPSPACE(s);
@@ -2397,41 +2387,63 @@ static rpmRC processPackageFiles(rpmSpec spec, 
rpmBuildPkgFlags pkgFlags,
rstrlcpy(buf, s, sizeof(buf));

/* Reset for a new line in %files */
-   FileEntryFree();
+   FileEntryFree(>cur);
 
/* turn explicit flags into %def'd ones (gosh this is hacky...) */
-   fl.cur.specdFlags = ((unsigned)fl.def.specdFlags) >> 8;
-   fl.cur.verifyFlags = fl.def.verifyFlags;
-
-   if (parseForVerify(buf, 0, ) ||
-   parseForVerify(buf, 1, ) ||
-   parseForAttr(fl.pool, buf, 0, ) ||
-   parseForAttr(fl.pool, buf, 1, ) ||
-   parseForDev(buf, ) ||
-   parseForConfig(buf, ) ||
-   parseForLang(buf, ) ||
-   parseForCaps(buf, ) ||
-   parseForSimple(buf, , ))
+   fl->cur.specdFlags = ((unsigned)fl->def.specdFlags) >> 8;
+   fl->cur.verifyFlags = fl->def.verifyFlags;
+
+   if (parseForVerify(buf, 0, >cur) ||
+   parseForVerify(buf, 1, >def) ||
+   parseForAttr(fl->pool, buf, 0, >cur) ||
+   parseForAttr(fl->pool, buf, 1, >def) ||
+   parseForDev(buf, >cur) ||
+   parseForConfig(buf, >cur) ||
+   parseForLang(buf, >cur) ||
+   parseForCaps(buf, >cur) ||
+   parseForSimple(buf, >cur, ))
{
-   fl.processingFailed = 1;
+   fl->processingFailed = 1;
continue;
}
 
for (ARGV_const_t fn = fileNames; fn && *fn; fn++) {
-   if (fl.cur.attrFlags & RPMFILE_SPECIALDIR) {
-   rpmFlags oattrs = (fl.cur.attrFlags & ~RPMFILE_SPECIALDIR);
+
+   /* For file lists 

[Rpm-maint] [PATCH] debugedit: skip_dir_prefix should check for dir separator.

[Mark Wielaard]

To count as a real directory prefix the string matched should either
be equal to the given prefix or start with the prefix plus '/'.

skip_dir_prefix is always used with base_dir or dest_dir which don't
end with a slash themselves.

This really only is an issue if a package would put a directory named
similar to the package source dir (which cargo on fedora does, by adding
a directory named cargo-vendor in the builddir itself).

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 tools/debugedit.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/debugedit.c b/tools/debugedit.c
index abd2ca4..a271b91 100644
--- a/tools/debugedit.c
+++ b/tools/debugedit.c
@@ -662,7 +662,8 @@ canonicalize_path (const char *s, char *d)
 /* Returns the rest of PATH if it starts with DIR_PREFIX, skipping any
/ path separators, or NULL if PATH doesn't start with
DIR_PREFIX. Might return the empty string if PATH equals DIR_PREFIX
-   (modulo trailing slashes). Never returns path starting with '/'. */
+   (modulo trailing slashes). Never returns path starting with '/'.
+   Note that DIR_PREFIX itself should NOT end with a '/'.  */
 static const char *
 skip_dir_prefix (const char *path, const char *dir_prefix)
 {
@@ -670,12 +671,17 @@ skip_dir_prefix (const char *path, const char *dir_prefix)
   if (strncmp (path, dir_prefix, prefix_len) == 0)
 {
   path += prefix_len;
+  /* Unless path == dir_prefix there should be at least one '/'
+in the path (which we will skip).  Otherwise the path has
+a different (longer) directory prefix.  */
+  if (*path != '\0' && !IS_DIR_SEPARATOR (*path))
+   return NULL;
   while (IS_DIR_SEPARATOR (path[0]))
path++;
   return path;
 }
 
-  return 0;
+  return NULL;
 }
 
 /* Most strings will be in the existing debug string table. But to
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] find-debuginfo.sh: Use 'return', not 'continue', to break out do_file().

[Mark Wielaard]

commit 038bfe "Split directory traversal and debuginfo extraction"
put the core of a while loop into its own function 'do_file()'.
That means that instead of using 'continue' to break out early it now
needs to use 'return'. Otherwise the script will give errors like:

  continue: only meaningful in a `for', `while', or `until' loop

https://bugzilla.redhat.com/show_bug.cgi?id=1465170

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 scripts/find-debuginfo.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/find-debuginfo.sh b/scripts/find-debuginfo.sh
index 360982d..59a4ff3 100755
--- a/scripts/find-debuginfo.sh
+++ b/scripts/find-debuginfo.sh
@@ -366,7 +366,7 @@ do_file()
   # just has its file names collected and adjusted.
   case "$dn" in
   /usr/lib/debug/*)
-continue ;;
+return ;;
   esac
 
   mkdir -p "${debugdn}"
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] find-debuginfo.sh: Filter out all like fake file names.

[Mark Wielaard]

There is no official way to mark an instruction range as being not
part of some actual source code, but as part of a compiler built-in
construct in DWARF. So different compilers have come up with fake
source file names like  or <__thread_local_inner macros>.
We already filtered out the strings "" and "".
Just filter out all '(^|/)<[a-z _-]+>$'. They are fake files!

This is mainly to appease the rustc compiler which generates lots of
different variants to encode some instruction sequence is part of an
compiler generated macro expansion.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 scripts/find-debuginfo.sh | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/scripts/find-debuginfo.sh b/scripts/find-debuginfo.sh
index 0233d92..555e7b8 100755
--- a/scripts/find-debuginfo.sh
+++ b/scripts/find-debuginfo.sh
@@ -502,7 +502,11 @@ if [ -s "$SOURCEFILE" ]; then
   fi
 
   mkdir -p "${RPM_BUILD_ROOT}${debug_dest_name}"
-  LC_ALL=C sort -z -u "$SOURCEFILE" | grep -E -v -z '(|)$' 
|
+  # Filter out anything compiler generated which isn't a source file.
+  # e.g. , , <__thread_local_inner macros>.
+  # Some compilers generate them as if they are part of the working
+  # directory (which is why we match against ^ or /).
+  LC_ALL=C sort -z -u "$SOURCEFILE" | grep -E -v -z '(^|/)<[a-z _-]+>$' |
   (cd "${debug_base_name}"; cpio -pd0mL "${RPM_BUILD_ROOT}${debug_dest_name}")
   # stupid cpio creates new directories in mode 0700,
   # and non-standard modes may be inherented from original directories, fixup
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] find-debuginfo.sh: Don't create dwz multi file if there is only one .debug.

[Mark Wielaard]

dwz -m multi only works when there are multiple .debug input files.
With just one .debug file it doesn't really make sense to extract
the shared debug info into a separate file and dwz will complain:

  dwz: Too few files for multifile optimization.

So only add -m multi if there is more than one .debug file.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 scripts/find-debuginfo.sh | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/scripts/find-debuginfo.sh b/scripts/find-debuginfo.sh
index aaf4c75..7ab39a2 100755
--- a/scripts/find-debuginfo.sh
+++ b/scripts/find-debuginfo.sh
@@ -442,8 +442,8 @@ fi
 # Invoke the DWARF Compressor utility.
 if $run_dwz \
&& [ -d "${RPM_BUILD_ROOT}/usr/lib/debug" ]; then
-  dwz_files="`cd "${RPM_BUILD_ROOT}/usr/lib/debug"; find -type f -name 
\*.debug`"
-  if [ -n "${dwz_files}" ]; then
+  readarray dwz_files < <(cd "${RPM_BUILD_ROOT}/usr/lib/debug"; find -type f 
-name \*.debug)
+  if [ ${#dwz_files[@]} -gt 0 ]; then
 
dwz_multifile_name="${RPM_PACKAGE_NAME}-${RPM_PACKAGE_VERSION}-${RPM_PACKAGE_RELEASE}.${RPM_ARCH}"
 dwz_multifile_suffix=
 dwz_multifile_idx=0
@@ -452,14 +452,16 @@ if $run_dwz \
   dwz_multifile_suffix=".${dwz_multifile_idx}"
 done
 dwz_multfile_name="${dwz_multifile_name}${dwz_multifile_suffix}"
-dwz_opts="-h -q -r -m .dwz/${dwz_multifile_name}"
+dwz_opts="-h -q -r"
+[ ${#dwz_files[@]} -gt 1 ] \
+  && dwz_opts="${dwz_opts} -m .dwz/${dwz_multifile_name}"
 mkdir -p "${RPM_BUILD_ROOT}/usr/lib/debug/.dwz"
 [ -n "${dwz_low_mem_die_limit}" ] \
   && dwz_opts="${dwz_opts} -l ${dwz_low_mem_die_limit}"
 [ -n "${dwz_max_die_limit}" ] \
   && dwz_opts="${dwz_opts} -L ${dwz_max_die_limit}"
 if type dwz >/dev/null 2>&1; then
-  ( cd "${RPM_BUILD_ROOT}/usr/lib/debug" && dwz $dwz_opts $dwz_files )
+  ( cd "${RPM_BUILD_ROOT}/usr/lib/debug" && dwz $dwz_opts ${dwz_files[@]} )
 else
   echo >&2 "*** ERROR: DWARF compression requested, but no dwz installed"
   exit 2
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] Update find-debuginfo.sh options and macros documentation.

[Mark Wielaard]

This adds some missing documentation for rpm macros and find-debuginfo.sh
options that were recently added (or renamed). -j N, --build-id-seed SEED,
--unique-debug-suffix SUFFIX and --unique-debug-src-base BASE.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 macros.in | 14 --
 scripts/find-debuginfo.sh | 28 ++--
 2 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/macros.in b/macros.in
index a2c15ad..78352a8 100644
--- a/macros.in
+++ b/macros.in
@@ -522,8 +522,9 @@ package or when debugging this package.\
 
 # Whether build-ids should be made unique between package version/releases
 # when generating debuginfo packages. If set to 1 this will pass
-# --ver-rel "%{VERSION}-%{RELEASE}" to find-debuginfo.sh which will pass it
-# onto debugedit --build-id-seed to be used to prime the build-id note hash.
+# --build-id-seed "%{VERSION}-%{RELEASE}" to find-debuginfo.sh which will
+# pass it onto debugedit --build-id-seed to be used to prime the build-id
+# note hash.
 %_unique_build_ids 1
 
 # Do not recompute build-ids but keep whatever is in the ELF file already.
@@ -533,15 +534,16 @@ package or when debugging this package.\
 
 # Whether .debug files should be made unique between package version,
 # release and architecture. If set to 1 this will pass
-# --unique-debug-arch "%{_arch}" to find-debuginfo.sh to create
-# debuginfo files which end in --..debug
+# --unique-debug-suffix "-%{VERSION}-%{RELEASE}.%{_arch} find-debuginfo.sh
+# to create debuginfo files which end in --..debug
 # Requires _unique_build_ids.
 %_unique_debug_names   1
 
 # Whether the /usr/debug/src/ directories should be unique between
 # package version, release and architecture. If set to 1 this will pass
-# --unique-debug-src-base "%{name}" to find-debuginfo.sh to name the
-# directory under /usr/debug/src as --.
+# --unique-debug-src-base "%{name}-%{VERSION}-%{RELEASE}.%{_arch}" to
+# find-debuginfo.sh to name the directory under /usr/debug/src as
+# --..
 %_unique_debug_srcs1
 
 # Whether rpm should put debug source files into its own subpackage
diff --git a/scripts/find-debuginfo.sh b/scripts/find-debuginfo.sh
index aaf4c75..0511777 100755
--- a/scripts/find-debuginfo.sh
+++ b/scripts/find-debuginfo.sh
@@ -3,11 +3,14 @@
 #for inclusion in an rpm spec file.
 #
 # Usage: find-debuginfo.sh [--strict-build-id] [-g] [-r] [-m] [-i] [-n]
+# [-j N]
 # [-o debugfiles.list]
 # [-S debugsourcefiles.list]
 # [--run-dwz] [--dwz-low-mem-die-limit N]
 # [--dwz-max-die-limit N]
-# [--build-id-seed VERSION-RELEASE]
+# [--build-id-seed SEED]
+# [--unique-debug-suffix SUFFIX]
+# [--unique-debug-src-base BASE]
 # [[-l filelist]... [-p 'pattern'] -o debuginfo.list]
 # [builddir]
 #
@@ -19,6 +22,9 @@
 # The -i flag says to include a .gdb_index section in the .debug file.
 # The -n flag says to not recompute the build-id.
 #
+# The -j N option will spawn N processes to do the debuginfo extraction
+# in parallel.
+#
 # A single -o switch before any -l or -p switches simply renames
 # the primary output file from debugfiles.list to something else.
 # A -o switch that follows a -p switch or some -l switches produces
@@ -31,11 +37,21 @@
 # if available, and --dwz-low-mem-die-limit and --dwz-max-die-limit
 # provide detailed limits.  See dwz(1) -l and -L option for details.
 #
-# If --build-id-seed VERSION-RELEASE is given then debugedit is called to
-# update the build-ids it finds adding the VERSION-RELEASE string as
-# seed to recalculate the build-id hash.  This makes sure the
-# build-ids in the ELF files are unique between versions and releases
-# of the same package.
+# If --build-id-seed SEED is given then debugedit is called to
+# update the build-ids it finds adding the SEED as seed to recalculate
+# the build-id hash.  This makes sure the build-ids in the ELF files
+# are unique between versions and releases of the same package.
+# (Use --build-id-seed "%{VERSION}-%{RELEASE}".)
+#
+# If --unique-debug-suffix SUFFIX is given then the debug files created
+# for  will be named -.debug.  This makes sure .debug
+# are unique between package version, release and architecture.
+# (Use --unique-debug-suffix "-%{VERSION}-%{RELEASE}.%{_arch}".)
+#
+# If --unique-debug-src-base BASE is given then the source directory
+# will be called /usr/debug/src/.  This makes sure the debug source
+# directories are unique between package version, release and architecture.
+# (Use --unique-debug-src-base "%{name}-%{VERSION}-%{RELEASE}.%{_arch}".)
 #
 # All file names in switches are relative to builddir (. if not given).
 #
-- 
1.8.3.1

Re: [Rpm-maint] [PATCH] Use a file list to add build-id files to pkgList and explicitly set attrs.

[Mark Wielaard]

Hi,

Panu sadi on irc he didn't like the duplication of code that parsed the
spec file lists. So this updated patch extracts the setup and parsing
loop in their own function and just calls them twice. I also reformatted
the patch a little so the whitespace differences are minimal.

Cheers,

Mark

From 739796798ac854f80ae2f0d677f74bca734055f7 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <m...@klomp.org>
Date: Wed, 21 Jun 2017 16:57:13 +0200
Subject: [PATCH] Use a file list to add build-id files to pkgList and
 explicitly set attrs.

mkattr used "-" as default mode which would pick up the mode for files
as they were on disk. This could cause files generated by rpmbuild to
use a "non-standard" mode if umask was set by the user. Explitictly
use 755 for directories and 644 for files to make builds independent
of any umask settings.

Change the generation of build-id files to a file list using ARGV_t.
First go through the current package list and generate a files list.
Then add those files using the defaults mode/attr settings as if they
were part of the original package file list.

https://bugzilla.redhat.com/show_bug.cgi?id=1452893
https://bugzilla.redhat.com/show_bug.cgi?id=1458839

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 build/files.c | 244 +-
 1 file changed, 138 insertions(+), 106 deletions(-)

diff --git a/build/files.c b/build/files.c
index 4911162d1..a90af3bee 100644
--- a/build/files.c
+++ b/build/files.c
@@ -207,9 +207,9 @@ static char *mkattr(const char *fn)
 {
 char *s = NULL;
 if (fn)
-	rasprintf(, "%s(-,%s,%s) %s", "%attr", UID_0_USER, GID_0_GROUP, fn);
+	rasprintf(, "%s(755,%s,%s) %s", "%attr", UID_0_USER, GID_0_GROUP, fn);
 else
-	rasprintf(, "%s(-,%s,%s)", "%defattr", UID_0_USER, GID_0_GROUP);
+	rasprintf(, "%s(644,%s,%s,755)", "%defattr", UID_0_USER, GID_0_GROUP);
 return s;
 }
 
@@ -1614,6 +1614,15 @@ exit:
 return rc;
 }
 
+/* add a directory to the file list */
+static void argvAddDir(ARGV_t *filesp, const char *dir)
+{
+char *line = NULL;
+rasprintf(, "%%dir %s", dir);
+argvAdd(filesp, line);
+_free(line);
+}
+
 #if HAVE_LIBDW
 /* How build id links are generated.  See macros.in for description.  */
 #define BUILD_IDS_NONE 0
@@ -1621,7 +1630,7 @@ exit:
 #define BUILD_IDS_SEPARATE 2
 #define BUILD_IDS_COMPAT   3
 
-static int addNewIDSymlink(FileList fl,
+static int addNewIDSymlink(ARGV_t *files,
 			   char *targetpath, char *idlinkpath,
 			   int isDbg, int *dups)
 {
@@ -1670,8 +1679,7 @@ static int addNewIDSymlink(FileList fl,
 	rpmlog(RPMLOG_ERR, "%s: %s -> %s: %m\n",
 	   linkerr, linkpath, targetpath);
 } else {
-	fl->cur.isDir = 0;
-	rc = addFile(fl, linkpath, NULL);
+	rc = argvAdd(files, linkpath);
 }
 
 if (nr > 0) {
@@ -1709,7 +1717,7 @@ static int addNewIDSymlink(FileList fl,
 return rc;
 }
 
-static int generateBuildIDs(FileList fl)
+static int generateBuildIDs(FileList fl, ARGV_t *files)
 {
 int rc = 0;
 int i;
@@ -1858,18 +1866,9 @@ static int generateBuildIDs(FileList fl)
 	mainiddir = rpmGetPath(fl->buildRoot, BUILD_ID_DIR, NULL);
 	debugiddir = rpmGetPath(fl->buildRoot, DEBUG_ID_DIR, NULL);
 
-	/* Make sure to reset all file flags to defaults.
-	   Uses parseForAttr to reset ar, arFlags, and specdFlags.
-	   Note that parseForAttr pokes at the attrstr, so we cannot
-	   just pass a static string. */
-	fl->cur.attrFlags = 0;
-	fl->def.attrFlags = 0;
-	fl->def.verifyFlags = RPMVERIFY_ALL;
-	fl->cur.verifyFlags = RPMVERIFY_ALL;
-	fl->def.specdFlags |= SPECD_VERIFY;
-	fl->cur.specdFlags |= SPECD_VERIFY;
+	/* Make sure to reset all file flags to defaults.  */
 	attrstr = mkattr(NULL);
-	parseForAttr(fl->pool, attrstr, 1, >def);
+	argvAdd(files, attrstr);
 	free (attrstr);
 
 	/* Supported, but questionable.  */
@@ -1881,11 +1880,7 @@ static int generateBuildIDs(FileList fl)
 		if ((rc = rpmioMkpath(mainiddir, 0755, -1, -1)) != 0) {
 		rpmlog(RPMLOG_ERR, "%s %s: %m\n", errdir, mainiddir);
 		} else {
-		attrstr = mkattr(mainiddir);
-		parseForAttr(fl->pool, attrstr, 0, >cur);
-		fl->cur.isDir = 1;
-		rc = addFile(fl, mainiddir, NULL);
-		free (attrstr);
+		argvAddDir(files, mainiddir);
 		}
 	}
 
@@ -1893,11 +1888,7 @@ static int generateBuildIDs(FileList fl)
 		if ((rc = rpmioMkpath(debugiddir, 0755, -1, -1)) != 0) {
 		rpmlog(RPMLOG_ERR, "%s %s: %m\n", errdir, debugiddir);
 		} else {
-		attrstr = mkattr(debugiddir);
-		parseForAttr(fl->pool, attrstr, 0, >cur);
-		fl->cur.isDir = 1;
-		rc = addFile(fl, debugiddir, NULL);
-		free (attrstr);
+		argvAddDir(files, debugiddir);
 		}
 	}
 	}
@@ -1936,9 +1927,

[Rpm-maint] [PATCH] Use a file list to add build-id files to pkgList and explicitly set attrs.

[Mark Wielaard]

When setting attributes with %attr or %defattr we want to be explicit
about the mode and not just use - to pick up the mode from the file
on disk.

Change the generation of build-id files to a file list using ARGV_t.
First go through the current package list and generate a files list.
Then add those files using the defaults mode/attr settings as if they
were part of the original package file list.

https://bugzilla.redhat.com/show_bug.cgi?id=1452893
https://bugzilla.redhat.com/show_bug.cgi?id=1458839

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 build/files.c | 310 +++---
 1 file changed, 167 insertions(+), 143 deletions(-)

diff --git a/build/files.c b/build/files.c
index 4911162..be72f0e 100644
--- a/build/files.c
+++ b/build/files.c
@@ -207,9 +207,9 @@ static char *mkattr(const char *fn)
 {
 char *s = NULL;
 if (fn)
-   rasprintf(, "%s(-,%s,%s) %s", "%attr", UID_0_USER, GID_0_GROUP, fn);
+   rasprintf(, "%s(755,%s,%s) %s", "%attr", UID_0_USER, GID_0_GROUP, fn);
 else
-   rasprintf(, "%s(-,%s,%s)", "%defattr", UID_0_USER, GID_0_GROUP);
+   rasprintf(, "%s(644,%s,%s,755)", "%defattr", UID_0_USER, GID_0_GROUP);
 return s;
 }
 
@@ -1614,6 +1614,15 @@ exit:
 return rc;
 }
 
+/* add a directory to the file list */
+static void argvAddDir(ARGV_t *filesp, const char *dir)
+{
+char *line = NULL;
+rasprintf(, "%%dir %s", dir);
+argvAdd(filesp, line);
+_free(line);
+}
+
 #if HAVE_LIBDW
 /* How build id links are generated.  See macros.in for description.  */
 #define BUILD_IDS_NONE 0
@@ -1621,7 +1630,7 @@ exit:
 #define BUILD_IDS_SEPARATE 2
 #define BUILD_IDS_COMPAT   3
 
-static int addNewIDSymlink(FileList fl,
+static int addNewIDSymlink(ARGV_t *files,
   char *targetpath, char *idlinkpath,
   int isDbg, int *dups)
 {
@@ -1670,8 +1679,7 @@ static int addNewIDSymlink(FileList fl,
rpmlog(RPMLOG_ERR, "%s: %s -> %s: %m\n",
   linkerr, linkpath, targetpath);
 } else {
-   fl->cur.isDir = 0;
-   rc = addFile(fl, linkpath, NULL);
+   rc = argvAdd(files, linkpath);
 }
 
 if (nr > 0) {
@@ -1709,7 +1717,7 @@ static int addNewIDSymlink(FileList fl,
 return rc;
 }
 
-static int generateBuildIDs(FileList fl)
+static int generateBuildIDs(FileList fl, ARGV_t *files)
 {
 int rc = 0;
 int i;
@@ -1858,18 +1866,9 @@ static int generateBuildIDs(FileList fl)
mainiddir = rpmGetPath(fl->buildRoot, BUILD_ID_DIR, NULL);
debugiddir = rpmGetPath(fl->buildRoot, DEBUG_ID_DIR, NULL);
 
-   /* Make sure to reset all file flags to defaults.
-  Uses parseForAttr to reset ar, arFlags, and specdFlags.
-  Note that parseForAttr pokes at the attrstr, so we cannot
-  just pass a static string. */
-   fl->cur.attrFlags = 0;
-   fl->def.attrFlags = 0;
-   fl->def.verifyFlags = RPMVERIFY_ALL;
-   fl->cur.verifyFlags = RPMVERIFY_ALL;
-   fl->def.specdFlags |= SPECD_VERIFY;
-   fl->cur.specdFlags |= SPECD_VERIFY;
+   /* Make sure to reset all file flags to defaults.  */
attrstr = mkattr(NULL);
-   parseForAttr(fl->pool, attrstr, 1, >def);
+   argvAdd(files, attrstr);
free (attrstr);
 
/* Supported, but questionable.  */
@@ -1881,11 +1880,7 @@ static int generateBuildIDs(FileList fl)
if ((rc = rpmioMkpath(mainiddir, 0755, -1, -1)) != 0) {
rpmlog(RPMLOG_ERR, "%s %s: %m\n", errdir, mainiddir);
} else {
-   attrstr = mkattr(mainiddir);
-   parseForAttr(fl->pool, attrstr, 0, >cur);
-   fl->cur.isDir = 1;
-   rc = addFile(fl, mainiddir, NULL);
-   free (attrstr);
+   argvAddDir(files, mainiddir);
}
}
 
@@ -1893,11 +1888,7 @@ static int generateBuildIDs(FileList fl)
if ((rc = rpmioMkpath(debugiddir, 0755, -1, -1)) != 0) {
rpmlog(RPMLOG_ERR, "%s %s: %m\n", errdir, debugiddir);
} else {
-   attrstr = mkattr(debugiddir);
-   parseForAttr(fl->pool, attrstr, 0, >cur);
-   fl->cur.isDir = 1;
-   rc = addFile(fl, debugiddir, NULL);
-   free (attrstr);
+   argvAddDir(files, debugiddir);
}
}
}
@@ -1936,114 +1927,113 @@ static int generateBuildIDs(FileList fl)
&& (rc = rpmioMkpath(buildidsubdir, 0755, -1, -1)) != 0) {
rpmlog(RPMLOG_ERR, "%s %s: %m\n", errdir, buildidsubdir);
  

Re: [Rpm-maint] [PATCH] rpmbuild: Reset attrFlags in generateBuildIDs.

[Mark Wielaard]

On Tue, May 23, 2017 at 02:01:46PM +0300, Panu Matilainen wrote:
> Hmm, I'm getting FAILED on the test:
> 
> --- - 2017-05-23 10:52:13.874228079 +
> +++ /home/pmatilai/repos/rpm/tests/rpmtests.dir/at-groups/144/stdout
> 2017-05-23 10:52:13.870950157 +
> @@ -1,2 +1,2 @@
> -/usr/local/etc/config.file
> +/etc/config.file
> 
> ...so probably because of different ./configure options to rpm itself that
> are inherited by the macro. %{_bindir}, %{_sysconfdir} etc are best avoided
> in the test-suite to avoid these issues.

O right of course. What about the attached which just uses /bin and /etc.
The actual path used doesn't really matter for the test and this should
work whatever --prefix you happen to configure with.

Thanks,

Mark
>From 396a52016dfde76a6d3c89b15d998a30341517b5 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <m...@klomp.org>
Date: Fri, 19 May 2017 23:11:39 +0200
Subject: [PATCH] rpmbuild: Reset attrFlags in generateBuildIDs.

Debuginfo directories and files could be marked as configuration files
if the file list ended with a config file.

Patch by Panu Matilainen. Testcase by me.

https://bugzilla.redhat.com/show_bug.cgi?id=1449732

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 build/files.c  |  2 ++
 tests/Makefile.am  |  1 +
 tests/data/SPECS/hello-config-buildid.spec | 30 
 tests/rpmbuildid.at| 32 ++
 4 files changed, 65 insertions(+)
 create mode 100644 tests/data/SPECS/hello-config-buildid.spec

diff --git a/build/files.c b/build/files.c
index 791bbe2..86c601b 100644
--- a/build/files.c
+++ b/build/files.c
@@ -1860,6 +1860,8 @@ static int generateBuildIDs(FileList fl)
   Uses parseForAttr to reset ar, arFlags, and specdFlags.
   Note that parseForAttr pokes at the attrstr, so we cannot
   just pass a static string. */
+   fl->cur.attrFlags = 0;
+   fl->def.attrFlags = 0;
fl->def.verifyFlags = RPMVERIFY_ALL;
fl->cur.verifyFlags = RPMVERIFY_ALL;
fl->def.specdFlags |= SPECD_VERIFY;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index a778af1..8670dcf 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -76,6 +76,7 @@ EXTRA_DIST += data/SRPMS/foo-1.0-1.src.rpm
 EXTRA_DIST += data/SRPMS/hello-1.0-1.src.rpm
 EXTRA_DIST += data/SOURCES/hello.c
 EXTRA_DIST += data/SPECS/hello-attr-buildid.spec
+EXTRA_DIST += data/SPECS/hello-config-buildid.spec
 EXTRA_DIST += data/keys/rpm.org-rsa-2048-test.pub
 EXTRA_DIST += data/keys/rpm.org-rsa-2048-test.secret
 
diff --git a/tests/data/SPECS/hello-config-buildid.spec 
b/tests/data/SPECS/hello-config-buildid.spec
new file mode 100644
index 000..ca2d30d
--- /dev/null
+++ b/tests/data/SPECS/hello-config-buildid.spec
@@ -0,0 +1,30 @@
+Name:   test
+Version:1.0
+Release:1
+Summary:Test
+
+License:Public Domain
+URL:https://fedoraproject.org
+Source: hello.c
+
+%description
+%{summary}.
+
+%prep
+%autosetup -c -D -T
+cp -a %{S:0} .
+
+%build
+gcc -g hello.c -o hello
+
+%install
+mkdir -p %{buildroot}/bin
+install -D -p -m 0755 -t %{buildroot}/bin hello
+mkdir -p %{buildroot}/etc
+echo "settings" > %{buildroot}/etc/config.file
+
+%files
+%attr(644,root,root) /bin/hello
+%config(noreplace) /etc/config.file
+
+%changelog
diff --git a/tests/rpmbuildid.at b/tests/rpmbuildid.at
index 88ce226..4fab3d5 100644
--- a/tests/rpmbuildid.at
+++ b/tests/rpmbuildid.at
@@ -1324,3 +1324,35 @@ run rpm -qp --qf "[[%{filenames} %{filemodes:perms}\n]]" 
\
 ],
 [ignore])
 AT_CLEANUP
+
+# --
+# Check that build-id directories are created with the right attributes
+# even if the spec file sets config explicitly.
+AT_SETUP([rpmbuild buildid config attrs])
+AT_KEYWORDS([build] [debuginfo] [buildid])
+AT_CHECK([
+rm -rf ${TOPDIR}
+AS_MKDIR_P(${TOPDIR}/SOURCES)
+
+# Setup sources
+cp "${abs_srcdir}"/data/SOURCES/hello.c ${TOPDIR}/SOURCES
+
+# Build, contains one ELF which should have a buildid.
+run rpmbuild \
+  
--macros=${abs_top_builddir}/macros:${abs_top_builddir}/tests/testing/usr/local/lib/rpm/platform/%{_target_cpu}-%{_target_os}/macros:${top_srcdir}/macros.debug
 \
+  --rcfile=${abs_top_builddir}/rpmrc \
+  --define="_build_id_links compat" \
+  --define "_unique_debug_names 1" \
+  --define "_unique_debug_srcs 1" \
+  --quiet -ba "${abs_srcdir}"/data/SPECS/hello-config-buildid.spec
+
+# Should contain one config file.
+run rpm -c -qp ${abs_builddir}/testing/build/RPMS/*/test-1.0-1*rpm
+# Should not contain config files.
+run rpm -c -qp ${abs_builddir}/testing/build/RPMS/*/test-debuginfo-1.0-1*rpm
+],
+[0],
+[/etc/config.file
+],
+[ignore])
+AT_CLEANUP
-- 
2.9.4

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] rpmbuild: Reset attrFlags in generateBuildIDs.

[Mark Wielaard]

Debuginfo directories and files could be marked as configuration files
if the file list ended with a config file.

Patch by Panu Matilainen. Testcase by me.

https://bugzilla.redhat.com/show_bug.cgi?id=1449732

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 build/files.c  |  2 ++
 tests/Makefile.am  |  1 +
 tests/data/SPECS/hello-config-buildid.spec | 30 
 tests/rpmbuildid.at| 32 ++
 4 files changed, 65 insertions(+)
 create mode 100644 tests/data/SPECS/hello-config-buildid.spec

diff --git a/build/files.c b/build/files.c
index 791bbe2..86c601b 100644
--- a/build/files.c
+++ b/build/files.c
@@ -1860,6 +1860,8 @@ static int generateBuildIDs(FileList fl)
   Uses parseForAttr to reset ar, arFlags, and specdFlags.
   Note that parseForAttr pokes at the attrstr, so we cannot
   just pass a static string. */
+   fl->cur.attrFlags = 0;
+   fl->def.attrFlags = 0;
fl->def.verifyFlags = RPMVERIFY_ALL;
fl->cur.verifyFlags = RPMVERIFY_ALL;
fl->def.specdFlags |= SPECD_VERIFY;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index a778af1..8670dcf 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -76,6 +76,7 @@ EXTRA_DIST += data/SRPMS/foo-1.0-1.src.rpm
 EXTRA_DIST += data/SRPMS/hello-1.0-1.src.rpm
 EXTRA_DIST += data/SOURCES/hello.c
 EXTRA_DIST += data/SPECS/hello-attr-buildid.spec
+EXTRA_DIST += data/SPECS/hello-config-buildid.spec
 EXTRA_DIST += data/keys/rpm.org-rsa-2048-test.pub
 EXTRA_DIST += data/keys/rpm.org-rsa-2048-test.secret
 
diff --git a/tests/data/SPECS/hello-config-buildid.spec 
b/tests/data/SPECS/hello-config-buildid.spec
new file mode 100644
index 000..346f266
--- /dev/null
+++ b/tests/data/SPECS/hello-config-buildid.spec
@@ -0,0 +1,30 @@
+Name:   test
+Version:1.0
+Release:1
+Summary:Test
+
+License:Public Domain
+URL:https://fedoraproject.org
+Source: hello.c
+
+%description
+%{summary}.
+
+%prep
+%autosetup -c -D -T
+cp -a %{S:0} .
+
+%build
+gcc -g hello.c -o hello
+
+%install
+mkdir -p %{buildroot}%{_bindir}
+install -D -p -m 0755 -t %{buildroot}%{_bindir} hello
+mkdir -p %{buildroot}%{_sysconfdir}
+echo "settings" > %{buildroot}%{_sysconfdir}/config.file
+
+%files
+%attr(644,root,root) %{_bindir}/hello
+%config(noreplace) %{_sysconfdir}/config.file
+
+%changelog
diff --git a/tests/rpmbuildid.at b/tests/rpmbuildid.at
index 88ce226..3511673 100644
--- a/tests/rpmbuildid.at
+++ b/tests/rpmbuildid.at
@@ -1324,3 +1324,35 @@ run rpm -qp --qf "[[%{filenames} %{filemodes:perms}\n]]" 
\
 ],
 [ignore])
 AT_CLEANUP
+
+# --
+# Check that build-id directories are created with the right attributes
+# even if the spec file sets config explicitly.
+AT_SETUP([rpmbuild buildid config attrs])
+AT_KEYWORDS([build] [debuginfo] [buildid])
+AT_CHECK([
+rm -rf ${TOPDIR}
+AS_MKDIR_P(${TOPDIR}/SOURCES)
+
+# Setup sources
+cp "${abs_srcdir}"/data/SOURCES/hello.c ${TOPDIR}/SOURCES
+
+# Build, contains one ELF which should have a buildid.
+run rpmbuild \
+  
--macros=${abs_top_builddir}/macros:${abs_top_builddir}/tests/testing/usr/local/lib/rpm/platform/%{_target_cpu}-%{_target_os}/macros:${top_srcdir}/macros.debug
 \
+  --rcfile=${abs_top_builddir}/rpmrc \
+  --define="_build_id_links compat" \
+  --define "_unique_debug_names 1" \
+  --define "_unique_debug_srcs 1" \
+  --quiet -ba "${abs_srcdir}"/data/SPECS/hello-config-buildid.spec
+
+# Should contain one config file.
+run rpm -c -qp ${abs_builddir}/testing/build/RPMS/*/test-1.0-1*rpm
+# Should not contain config files.
+run rpm -c -qp ${abs_builddir}/testing/build/RPMS/*/test-debuginfo-1.0-1*rpm
+],
+[0],
+[/usr/local/etc/config.file
+],
+[ignore])
+AT_CLEANUP
-- 
2.9.3

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] debugedit: Only output comp_dir under build dir (once).

[Mark Wielaard]

The fix for rhbz#444310 (commit c1a5eb - Include empty CU current dirs)
was a little greedy. It would also include comp_dirs outside the build
root. Those are unnecessary and we don't have a good way to store them.
Such dirs (e.g. /tmp) would then show up at the root of /usr/src/debug.

Fix this by including only comp_dirs under base_dir. Also only output
all dirs once (during phase zero) and don't output empty dirs (which
was harmless but would produce a warning from cpio).

This still includes all empty dirs from the original rhbz#444310
nodir testcase and it is an alternative fix for rhbz#641022
(commit c707ab).

Both fixes are necessary in case of an unexpected mode for a directory
actually in the build root that we want to include in the source list.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 tools/debugedit.c | 39 ---
 1 file changed, 16 insertions(+), 23 deletions(-)

diff --git a/tools/debugedit.c b/tools/debugedit.c
index 8444e03..bf11513 100644
--- a/tools/debugedit.c
+++ b/tools/debugedit.c
@@ -1680,30 +1680,23 @@ edit_attributes (DSO *dso, unsigned char *ptr, struct 
abbrev_tag *t, int phase)
   /* Ensure the CU current directory will exist even if only empty.  Source
  filenames possibly located in its parent directories refer relatively to
  it and the debugger (GDB) cannot safely optimize out the missing
- CU current dir subdirectories.  */
-  if (comp_dir && list_file_fd != -1)
+ CU current dir subdirectories.  Only do this once in phase one. And
+ only do this for dirs under our build/base_dir.  Don't output the
+ empty string (in case the comp_dir == base_dir).  */
+  if (phase == 0 && base_dir && comp_dir && list_file_fd != -1)
 {
-  const char *p = NULL;
-  size_t size;
-
-  if (base_dir)
-   {
- p = skip_dir_prefix (comp_dir, base_dir);
- if (p == NULL && dest_dir != NULL)
-   p = skip_dir_prefix (comp_dir, dest_dir);
-   }
-
-  if (p == NULL)
-   p = comp_dir;
-
-  size = strlen (p) + 1;
-  while (size > 0)
-   {
- ssize_t ret = write (list_file_fd, p, size);
- if (ret == -1)
-   break;
- size -= ret;
- p += ret;
+  const char *p = skip_dir_prefix (comp_dir, base_dir);
+  if (p != NULL && p[0] != '\0')
+{
+ size_t size = strlen (p) + 1;
+ while (size > 0)
+   {
+ ssize_t ret = write (list_file_fd, p, size);
+ if (ret == -1)
+   break;
+ size -= ret;
+ p += ret;
+   }
}
 }
 
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] debugedit: Add -n, --no-recompute-build-id.

[Mark Wielaard]

Some packages depend on the build-ids as generated during the build
and cannot handle rpmbuild recomputing them before generating the
package file list. Add -n, --no-recompute-build-id to debugedit and
add -n to find-debuginfo.sh set by defining the %_no_recompute_build_ids
macro for such packages. %_no_recompute_build_ids can not be used together
with %_unique_build_ids.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 macros.in |   7 ++-
 scripts/find-debuginfo.sh |  20 +++-
 tests/rpmbuildid.at   | 122 ++
 tools/debugedit.c |   6 ++-
 4 files changed, 151 insertions(+), 4 deletions(-)

diff --git a/macros.in b/macros.in
index f7d16de..cf22628 100644
--- a/macros.in
+++ b/macros.in
@@ -172,7 +172,7 @@
 #  the script.  See the script for details.
 #
 %__debug_install_post   \
-   %{_rpmconfigdir}/find-debuginfo.sh %{?_smp_mflags} 
%{?_missing_build_ids_terminate_build:--strict-build-id} 
%{?_include_minidebuginfo:-m} %{?_include_gdb_index:-i} 
%{?_unique_build_ids:--ver-rel "%{VERSION}-%{RELEASE}"} 
%{?_unique_debug_names:--unique-debug-arch "%{_arch}"} 
%{?_unique_debug_srcs:--unique-debug-src-base "%{name}"} 
%{?_find_debuginfo_dwz_opts} %{?_find_debuginfo_opts} 
"%{_builddir}/%{?buildsubdir}"\
+   %{_rpmconfigdir}/find-debuginfo.sh %{?_smp_mflags} 
%{?_missing_build_ids_terminate_build:--strict-build-id} 
%{?_no_recompute_build_ids:-n} %{?_include_minidebuginfo:-m} 
%{?_include_gdb_index:-i} %{?_unique_build_ids:--ver-rel 
"%{VERSION}-%{RELEASE}"} %{?_unique_debug_names:--unique-debug-arch "%{_arch}"} 
%{?_unique_debug_srcs:--unique-debug-src-base "%{name}"} 
%{?_find_debuginfo_dwz_opts} %{?_find_debuginfo_opts} 
"%{_builddir}/%{?buildsubdir}"\
 %{nil}
 
 #  Template for debug information sub-package.
@@ -498,6 +498,11 @@ package or when debugging this package.\
 # onto debugedit --build-id-seed to be used to prime the build-id note hash.
 %_unique_build_ids 1
 
+# Do not recompute build-ids but keep whatever is in the ELF file already.
+# Cannot be used together with _unique_build_ids (which forces recomputation).
+# Defaults to undefined (unset).
+#%_no_recompute_build_ids 1
+
 # Whether .debug files should be made unique between package version,
 # release and architecture. If set to 1 this will pass
 # --unique-debug-arch "%{_arch}" to find-debuginfo.sh to create
diff --git a/scripts/find-debuginfo.sh b/scripts/find-debuginfo.sh
index 39bb0b7..8effac8 100755
--- a/scripts/find-debuginfo.sh
+++ b/scripts/find-debuginfo.sh
@@ -2,7 +2,7 @@
 #find-debuginfo.sh - automagically generate debug info and file list
 #for inclusion in an rpm spec file.
 #
-# Usage: find-debuginfo.sh [--strict-build-id] [-g] [-r] [-m] [-i]
+# Usage: find-debuginfo.sh [--strict-build-id] [-g] [-r] [-m] [-i] [-n]
 # [-o debugfiles.list]
 # [--run-dwz] [--dwz-low-mem-die-limit N]
 # [--dwz-max-die-limit N]
@@ -16,6 +16,7 @@
 # The -r flag says to use eu-strip --reloc-debug-sections.
 # The -m flag says to include a .gnu_debugdata section in the main binary.
 # The -i flag says to include a .gdb_index section in the .debug file.
+# The -n flag says to not recompute the build-id.
 #
 # A single -o switch before any -l or -p switches simply renames
 # the primary output file from debugfiles.list to something else.
@@ -56,6 +57,9 @@ include_gdb_index=false
 # Barf on missing build IDs.
 strict=false
 
+# Do not recompute build IDs.
+no_recompute_build_id=false
+
 # DWZ parameters.
 run_dwz=false
 dwz_low_mem_die_limit=
@@ -110,6 +114,9 @@ while [ $# -gt 0 ]; do
   -m)
 include_minidebug=true
 ;;
+  -n)
+no_recompute_build_id=true
+;;
   -i)
 include_gdb_index=true
 ;;
@@ -159,6 +166,11 @@ if test -z "$unique_debug_arch" -a -n 
"$unique_debug_src_base"; then
   exit 2
 fi
 
+if test -n "$ver_rel" -a "$no_recompute_build_id" = "true"; then
+  echo >&2 "*** ERROR: --ver-rel (unique build-ids) and -n (do not recompute 
build-id cannot be used together"
+  exit 2
+fi
+
 i=0
 while ((i < nout)); do
   outs[$i]="$BUILDDIR/${outs[$i]}"
@@ -324,8 +336,12 @@ do_file()
 debug_base_name="$BUILDDIR"
 
debug_dest_name="/usr/src/debug/${unique_debug_src_base}-${ver_rel}.${unique_debug_arch}"
   fi
+  no_recompute=
+  if [ "$no_recompute_build_id" = "true" ]; then
+no_recompute="-n"
+  fi
   id=$(${lib_rpm_dir}/debugedit -b $debug_base_name -d $debug_dest_name \
- -i $build_id_seed -l "$SOURCEFILE" "$f") || exit
+   $no_recompute -i $build_id_seed -l "$SOURCEFILE" "$f") || exit
   if [ -z "$id" ]; then
 echo >&

[Rpm-maint] [PATCH] find-debuginfo.sh: Only add minisymtab for executables or shared libraries.

[Mark Wielaard]

It only makes sense to add a minisymtab for executables and shared
libraries. Other executable ELF files (like kernel modules) don't need it.
Since those don't have a dynsym section trying to add it will fail and
produce confusing errors from nm.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 scripts/find-debuginfo.sh | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/scripts/find-debuginfo.sh b/scripts/find-debuginfo.sh
index 6f38e19..39bb0b7 100755
--- a/scripts/find-debuginfo.sh
+++ b/scripts/find-debuginfo.sh
@@ -358,7 +358,16 @@ do_file()
   fi
 
   # strip -g implies we have full symtab, don't add mini symtab in that case.
-  $strip_g || ($include_minidebug && add_minidebug "${debugfn}" "$f")
+  # It only makes sense to add a minisymtab for executables and shared
+  # libraries. Other executable ELF files (like kernel modules) don't need it.
+  if [ "$include_minidebug" = "true" -a "$strip_g" = "false" ]; then
+skip_mini=true
+case "$(file -bi "$f")" in
+  application/x-sharedlib*) skip_mini=false ;;
+  application/x-executable*) skip_mini=false ;;
+esac
+$skip_mini || add_minidebug "${debugfn}" "$f"
+  fi
 
   echo "./${f#$RPM_BUILD_ROOT}" >> "$ELFBINSFILE"
 
-- 
2.9.3

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] Make rpmsign tests work for builddir != srcdir.

[Mark Wielaard]

The gpg HOME is in the builddir testing directory.
But the keys to import are in the srcdir data/keys directory.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 tests/Makefile.am | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 6adc709..5fa06eb 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -110,7 +110,7 @@ EXTRA_DIST += atlocal.in
 # Hack: Abusing testing$(bindir)/rpmbuild as stamp file
 # The chmod is needed when copying from read-only sources (eg in distcheck)
 testing$(bindir)/rpmbuild: ../rpmbuild
-   HOME=$(abs_srcdir)/testing gpg-connect-agent --no-autostart killagent 
bye ||:
+   HOME=$(abs_builddir)/testing gpg-connect-agent --no-autostart killagent 
bye ||:
rm -rf testing
mkdir -p testing/$(bindir)
ln -s ./$(bindir) testing/bin
@@ -126,7 +126,7 @@ testing$(bindir)/rpmbuild: ../rpmbuild
for prog in gzip cat patch tar sh ln chmod rm mkdir uname grep sed find 
file ionice mktemp nice cut sort diff touch; do p=`which $${prog}`; ln -s $${p} 
testing/$(bindir)/; done
for d in /proc /sys /selinux /etc/selinux; do if [ -d $${d} ]; then ln 
-s $${d} testing/$${d}; fi; done
(cd testing/magic && file -C)
-   HOME=$(abs_srcdir)/testing gpg2 --import data/keys/*.secret || 
HOME=$(abs_srcdir)/testing gpg --import data/keys/*.secret
+   HOME=$(abs_builddir)/testing gpg2 --import 
${abs_srcdir}/data/keys/*.secret || HOME=$(abs_builddir)/testing gpg --import 
${abs_srcdir}/data/keys/*.secret
 
 check_DATA = atconfig atlocal $(TESTSUITE)
 check_DATA += testing$(bindir)/rpmbuild
-- 
2.9.3

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [PATCH 2/5] Only build bundled fts if system has a bad version that doesn't handle LFS

[Mark Wielaard]

On Fri, 2017-03-24 at 11:15 +0200, Panu Matilainen wrote:
> On 03/23/2017 08:21 PM, Gleb Fotengauer-Malinovskiy wrote:
> > diff --git a/configure.ac b/configure.ac
> > index bdcb741..687d58c 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -717,6 +717,9 @@ AC_CHECK_FUNCS(
> > [mkstemp getcwd basename dirname realpath setenv unsetenv regcomp 
> > lchown utimes],
> > [], [AC_MSG_ERROR([function required by rpm])])
> >
> > +AC_CHECK_HEADERS([fts.h])
> > +AM_CONDITIONAL([USE_BUNDLED_FTS_KLUDGE], [test "$ac_cv_header_fts_h" = no])
> > +
> >  AC_LIBOBJ(fnmatch)
> >
> 
> How exactly does this ensure the system fts.h is LFS compatible?

The configure.ac has an AC_SYS_LARGEFILE fairly early. Which will define
_FILE_OFFSET_BITS 64 on 32-bit systems that need it. So on such systems
when AC_CHECK_HEADERS([fts.h]) is ran it will have that define in the
conftest.c and the test compile will fail because old fts.h have an
explicit check (error out on) -D_FILE_OFFSET_BITS==64.

Cheers,

Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] build/files.c (generateBuildIDs): Fix small memory leak.

[Mark Wielaard]

From: Mark Wielaard <m...@klomp.org>

mainiddir and debugiddir are allocated through rpmGetPath ()
and should be released when done.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 build/files.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/build/files.c b/build/files.c
index 93021d1..d7b140e 100644
--- a/build/files.c
+++ b/build/files.c
@@ -2022,6 +2022,8 @@ static int generateBuildIDs(FileList fl)
free(paths[i]);
free(ids[i]);
}
+   free(mainiddir);
+   free(debugiddir);
free(vra);
free(paths);
free(ids);
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [PATCH v2] Unbreak short-circuited binary builds (RhBug:1434235)

[Mark Wielaard]

On Wed, 2017-03-22 at 14:37 +0200, Panu Matilainen wrote:
> Guess there's some unhandled corner left still :)

Yeah, that was because my fix was bogus :{

It didn't construct the linkpath correctly (because it was relative). It
checked with readlink (), but that doesn't fully expand the links, it
should have used canonicalize_file_name () on both the link and the
target (because the target is itself a symlink). And even then it
wouldn't have worked because we don't know which duplicate the compat
link is targeting, so the first existing target would always win.

So back to the drawing board. The problem is the compat links in case
there are duplicate build-ids. We don't want the compat links to
"duplicate" the duplication search already done for the buildids already
done. So in this new patch instead of saying we want to create a compat
link when calling addNewBuildIDSymlink, we ask to search (and return)
the number of duplicates found. We can then use that to create the
compat links directly (with the correct duplication counter).

Attached is your original patch plus my don't do duplication search for
compat links change. That makes the testsuite pass and the popt
short-circuit example work for me.

Cheers,

Mark
From e6c0dbf9c6e56bfae6a596929b69d4474c3d69f1 Mon Sep 17 00:00:00 2001
From: Panu Matilainen <pmati...@redhat.com>
Date: Thu, 5 Jan 2017 13:47:28 +0200
Subject: [PATCH] Unbreak short-circuited binary builds

Commit bbfe1f86b2e4b5c0bd499d9f3dd9de9c9c20fff2 broke short-circuited
binary builds (which can be handy for testing when working on large
packages), eg:
 rpmbuild -bi foo.spec; rpmbuild -bb --short-circuit foo.spec

The problem is that in a short-circuited build all the links already
exist and point to the right place, but the code doesn't realize this
and creates new links instead, which leaves the old links unowned
in the buildroot which ultimately causes the build to fail with
"Installed (but unpackaged) file(s) found" for the previously created
build-id links.

When checking for pre-existing links see if they already point to
the right file and in that case just reuse it instead of creating new ones.
Keep track of duplicate build-ids found by noticing existing links that
point to different targets. But don't do this for compat links, they should
just point to the last (duplicate) main build-id symlink found.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 build/files.c | 71 ++-
 1 file changed, 51 insertions(+), 20 deletions(-)

diff --git a/build/files.c b/build/files.c
index cca14b9..93021d1 100644
--- a/build/files.c
+++ b/build/files.c
@@ -1592,11 +1592,12 @@ exit:
 
 static int addNewIDSymlink(FileList fl,
 			   char *targetpath, char *idlinkpath,
-			   int isDbg, int isCompat)
+			   int isDbg, int *dups)
 {
 const char *linkerr = _("failed symlink");
 int rc = 0;
 int nr = 0;
+int exists = 0;
 char *origpath, *linkpath;
 
 if (isDbg)
@@ -1606,6 +1607,26 @@ static int addNewIDSymlink(FileList fl,
 origpath = linkpath;
 
 while (faccessat(AT_FDCWD, linkpath, F_OK, AT_SYMLINK_NOFOLLOW) == 0) {
+/* We don't care about finding dups for compat links, they are
+	   OK as is.  Otherwise we will need to double check if
+	   existing link points to the correct target. */
+	if (dups == NULL)
+	  {
+	exists = 1;
+	break;
+	  }
+
+	char ltarget[PATH_MAX];
+	ssize_t llen;
+	/* In short-circuited builds the link might already exist  */
+	if ((llen = readlink(linkpath, ltarget, sizeof(ltarget)-1)) != -1) {
+	ltarget[llen] = '\0';
+	if (rstreq(ltarget, targetpath)) {
+		exists = 1;
+		break;
+	}
+	}
+
 	if (nr > 0)
 	free(linkpath);
 	nr++;
@@ -1613,21 +1634,16 @@ static int addNewIDSymlink(FileList fl,
 		  isDbg ? ".debug" : "");
 }
 
-char *symtarget = targetpath;
-if (nr > 0 && isCompat)
-	rasprintf (, "%s.%d", targetpath, nr);
-
-if (symlink(symtarget, linkpath) < 0) {
+if (!exists && symlink(targetpath, linkpath) < 0) {
 	rc = 1;
 	rpmlog(RPMLOG_ERR, "%s: %s -> %s: %m\n",
-	   linkerr, linkpath, symtarget);
+	   linkerr, linkpath, targetpath);
 } else {
 	fl->cur.isDir = 0;
 	rc = addFile(fl, linkpath, NULL);
 }
 
-/* Don't warn (again) if this is a compat id-link, we retarget it. */
-if (nr > 0 && !isCompat) {
+if (nr > 0) {
 	/* Lets see why there are multiple build-ids. If the original
 	   targets are hard linked, then it is OK, otherwise warn
 	   something fishy is going on. Would be nice to call
@@ -1656,8 +1672,8 @@ static int addNewIDSymlink(FileList fl,
 	free(origpath);
 if (nr > 0)
 	free(linkpath);
-if (nr > 0 && isCompat)
-	free(symtarget);
+if (dups != NULL)
+  *dups = nr;
 
 return rc;
 }
@@ -1897,6 +1913,7 @@ static int generate

[Rpm-maint] [PATCH] build/files.c: Only check build-ids for executable files in the main package.

[Mark Wielaard]

From: Mark Wielaard <m...@klomp.org>

generateBuildIDs should mimic what find-debuginfo.sh does. Only check
build-ids for executable files in non-debuginfo packages. This moves the
isDbg check up so the is executeble check can be done when the file is
part of the main package.

This fixes the build of qemu and uboot-tools in fedora. Both ship
non-executable ELF bios files in architecture specific packages.
https://bugzilla.redhat.com/show_bug.cgi?id=1433837

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 build/files.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/build/files.c b/build/files.c
index 6021643..afa01cd 100644
--- a/build/files.c
+++ b/build/files.c
@@ -1711,6 +1711,19 @@ static int generateBuildIDs(FileList fl)
 for (i = 0, flp = fl->files.recs; i < fl->files.used; i++, flp++) {
struct stat sbuf;
if (lstat(flp->diskPath, ) == 0 && S_ISREG (sbuf.st_mode)) {
+   /* We determine whether this is a main or
+  debug ELF based on path.  */
+   #define DEBUGPATH "/usr/lib/debug/"
+   int isDbg = strncmp (flp->cpioPath,
+DEBUGPATH, strlen (DEBUGPATH)) == 0;
+
+   /* For the main package files mimic what find-debuginfo.sh does.
+  Only check build-ids for executable files. Debug files are
+  always non-executable. */
+   if (!isDbg
+   && (sbuf.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH)) == 0)
+ continue;
+
int fd = open (flp->diskPath, O_RDONLY);
if (fd >= 0) {
/* Only real ELF files, that are ET_EXEC, ET_DYN or
@@ -1732,12 +1745,8 @@ static int generateBuildIDs(FileList fl)
   is 128 bits) and 64 bytes (largest sha3 is 512
   bits), common is 20 bytes (sha1 is 160 bits). */
if (len >= 16 && len <= 64) {
-   /* We determine whether this is a main or
-  debug ELF based on path.  */
-   #define DEBUGPATH "/usr/lib/debug/"
int addid = 0;
-   if (strncmp (flp->cpioPath,
-DEBUGPATH, strlen (DEBUGPATH)) == 0) {
+   if (isDbg) {
needDbg = 1;
addid = 1;
}
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] tests/rpmbuildid.at: Make file sed regexp more strict to extract BuildID.

[Mark Wielaard]

From: Mark Wielaard <m...@klomp.org>

Like commit f0a5819 for rpmbuild.at. In the case of rpmbuildid.at the
sed expression looked to work, but only matched by accident. Make the sed
regexp more strict by only matching a hex-string. And properly "escape"
[ and ] which inside an AT_CHECK should be [[ and ]].

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 tests/rpmbuildid.at | 44 ++--
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/tests/rpmbuildid.at b/tests/rpmbuildid.at
index 15c0620..1c06ca1 100644
--- a/tests/rpmbuildid.at
+++ b/tests/rpmbuildid.at
@@ -97,7 +97,7 @@ main_file=./usr/local/bin/hello
 test -f "${main_file}" || echo "No main file ${main_file}"
 
 # Extract the build-id from the main file
-id_main=$(file $main_file | sed 's/.*, BuildID[.*]=\(.*\),.*/\1/')
+id_main=$(file $main_file | sed 's/.*, BuildID\[[.*\]]=\([[0-9a-f]]*\),.*/\1/')
 
 id_main_file="./usr/lib/debug/.build-id/${id_main:0:2}/${id_main:2}"
 test -L "$id_main_file" || echo "No build-id file $id_main_file"
@@ -120,7 +120,7 @@ debug_file=./usr/lib/debug/usr/local/bin/hello.debug
 test -f ${debug_file} || echo "No debug file ${debug_file}"
 
 # Extract the build-id from the .debug file
-id_debug=$(file $debug_file | sed 's/.*, BuildID[.*]=\(.*\),.*/\1/')
+id_debug=$(file $debug_file | sed 's/.*, 
BuildID\[[.*\]]=\([[0-9a-f]]*\),.*/\1/')
 
 test ${id_main} = ${id_debug} || echo "unequal main and debug id"
 
@@ -190,7 +190,7 @@ main_file=./usr/local/bin/hello
 test -f "${main_file}" || echo "No main file ${main_file}"
 
 # Extract the build-id from the main file
-id_main=$(file $main_file | sed 's/.*, BuildID[.*]=\(.*\),.*/\1/')
+id_main=$(file $main_file | sed 's/.*, BuildID\[[.*\]]=\([[0-9a-f]]*\),.*/\1/')
 
 id_main_file="./usr/lib/debug/.build-id/${id_main:0:2}/${id_main:2}"
 test -L "$id_main_file" || echo "No build-id file $id_main_file"
@@ -213,7 +213,7 @@ debug_file=./usr/lib/debug/usr/local/bin/hello-*.debug
 test -f ${debug_file} || echo "No debug file ${debug_file}"
 
 # Extract the build-id from the .debug file
-id_debug=$(file $debug_file | sed 's/.*, BuildID[.*]=\(.*\),.*/\1/')
+id_debug=$(file $debug_file | sed 's/.*, 
BuildID\[[.*\]]=\([[0-9a-f]]*\),.*/\1/')
 
 test ${id_main} = ${id_debug} || echo "unequal main and debug id"
 
@@ -283,7 +283,7 @@ main_file=./usr/local/bin/hello
 test -f "${main_file}" || echo "No main file ${main_file}"
 
 # Extract the build-id from the main file
-id_main=$(file $main_file | sed 's/.*, BuildID[.*]=\(.*\),.*/\1/')
+id_main=$(file $main_file | sed 's/.*, BuildID\[[.*\]]=\([[0-9a-f]]*\),.*/\1/')
 
 id_main_file="./usr/lib/.build-id/${id_main:0:2}/${id_main:2}"
 test -L "$id_main_file" || echo "No build-id file $id_main_file"
@@ -306,7 +306,7 @@ debug_file=./usr/lib/debug/usr/local/bin/hello.debug
 test -f ${debug_file} || echo "No debug file ${debug_file}"
 
 # Extract the build-id from the .debug file
-id_debug=$(file $debug_file | sed 's/.*, BuildID[.*]=\(.*\),.*/\1/')
+id_debug=$(file $debug_file | sed 's/.*, 
BuildID\[[.*\]]=\([[0-9a-f]]*\),.*/\1/')
 
 test ${id_main} = ${id_debug} || echo "unequal main and debug id"
 
@@ -375,7 +375,7 @@ main_file=./usr/local/bin/hello
 test -f "${main_file}" || echo "No main file ${main_file}"
 
 # Extract the build-id from the main file
-id_main=$(file $main_file | sed 's/.*, BuildID[.*]=\(.*\),.*/\1/')
+id_main=$(file $main_file | sed 's/.*, BuildID\[[.*\]]=\([[0-9a-f]]*\),.*/\1/')
 
 id_main_file="./usr/lib/.build-id/${id_main:0:2}/${id_main:2}"
 test -L "$id_main_file" || echo "No build-id file $id_main_file"
@@ -398,7 +398,7 @@ debug_file=./usr/lib/debug/usr/local/bin/hello-*.debug
 test -f ${debug_file} || echo "No debug file ${debug_file}"
 
 # Extract the build-id from the .debug file
-id_debug=$(file $debug_file | sed 's/.*, BuildID[.*]=\(.*\),.*/\1/')
+id_debug=$(file $debug_file | sed 's/.*, 
BuildID\[[.*\]]=\([[0-9a-f]]*\),.*/\1/')
 
 test ${id_main} = ${id_debug} || echo "unequal main and debug id"
 
@@ -469,7 +469,7 @@ main_file=./usr/local/bin/hello
 test -f "${main_file}" || echo "No main file ${main_file}"
 
 # Extract the build-id from the main file
-id_main=$(file $main_file | sed 's/.*, BuildID[.*]=\(.*\),.*/\1/')
+id_main=$(file $main_file | sed 's/.*, BuildID\[[.*\]]=\([[0-9a-f]]*\),.*/\1/')
 
 id_main_file="./usr/lib/.build-id/${id_main:0:2}/${id_main:2}"
 test -L "$id_main_file" || echo "No build-id file $id_main_file"
@@ -492,7 +492,7 @@ debug_file=./usr/lib/debug/usr/local/bin/hello.debug
 test -f ${debug_file} || echo "No debug file ${debug_file}"
 
 # Extract the build-id from the .debug file
-id_d

[Rpm-maint] [PATCH] debugedit: Fix cross-endian build-id reading and updating section data.

[Mark Wielaard]

From: Mark Wielaard <m...@klomp.org>

debugedit doesn't read raw mmap data any longer. Which made the complex
way to read the build-id unnecessary (and it was broken for cross-endian).
Just use gelf_getnote to read the notes.

Also in some special cases when only the debug_info or build_id data
was updated, but no section changed size and we had to preserve the
allocated section headers we could hit a bug in elfutils that could
trash some section data in case there were gaps between non-dirty and
dirty sections. See https://sourceware.org/bugzilla/show_bug.cgi?id=21199
Add a workaround for that issue.

This fixes the kompose package build on fedora ppc64.
And makes it possible to replicate that issue on x86_64.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 tools/debugedit.c | 63 +++
 1 file changed, 31 insertions(+), 32 deletions(-)

diff --git a/tools/debugedit.c b/tools/debugedit.c
index 4798c63..47e5bbf 100644
--- a/tools/debugedit.c
+++ b/tools/debugedit.c
@@ -2581,40 +2581,25 @@ main (int argc, char *argv[])
  break;
case SHT_NOTE:
  if (do_build_id
- && build_id == NULL && (dso->shdr[i].sh_flags & SHF_ALLOC))
+ && build_id == 0 && (dso->shdr[i].sh_flags & SHF_ALLOC))
{
  /* Look for a build-ID note here.  */
+ size_t off = 0;
+ GElf_Nhdr nhdr;
+ size_t name_off;
+ size_t desc_off;
  Elf_Data *data = elf_getdata (elf_getscn (dso->elf, i), NULL);
- Elf32_Nhdr nh;
- Elf_Data dst =
-   {
- .d_version = EV_CURRENT, .d_type = ELF_T_NHDR,
- .d_buf = , .d_size = sizeof nh
-   };
- Elf_Data src = dst;
- src.d_buf = data->d_buf;
- assert (sizeof (Elf32_Nhdr) == sizeof (Elf64_Nhdr));
- while ((char *) data->d_buf + data->d_size -
-(char *) src.d_buf > (int) sizeof nh
-&& elf32_xlatetom (, , dso->ehdr.e_ident[EI_DATA]))
-   {
- Elf32_Word len = sizeof nh + nh.n_namesz;
- len = (len + 3) & ~3;
-
- if (nh.n_namesz == sizeof "GNU" && nh.n_type == 3
- && !memcmp ((char *) src.d_buf + sizeof nh, "GNU", sizeof 
"GNU"))
-   {
- build_id = data;
- build_id_offset = (char *) src.d_buf + len -
-   (char *) data->d_buf;
- build_id_size = nh.n_descsz;
- break;
-   }
-
- len += nh.n_descsz;
- len = (len + 3) & ~3;
- src.d_buf = (char *) src.d_buf + len;
-   }
+ while ((off = gelf_getnote (data, off,
+ , _off, _off)) > 0)
+   if (nhdr.n_type == NT_GNU_BUILD_ID
+   && nhdr.n_namesz == sizeof "GNU"
+   && (memcmp ((char *)data->d_buf + name_off, "GNU",
+   sizeof "GNU") == 0))
+ {
+   build_id = data;
+   build_id_offset = desc_off;
+   build_id_size = nhdr.n_descsz;
+ }
}
  break;
default:
@@ -2622,6 +2607,20 @@ main (int argc, char *argv[])
}
 }
 
+  /* Normally we only need to explicitly update the section headers
+ and data when any section data changed size. But because of a bug
+ in elfutils before 0.169 we will have to update and write out all
+ section data if any data has changed (when ELF_F_LAYOUT was
+ set). https://sourceware.org/bugzilla/show_bug.cgi?id=21199 */
+  bool need_update = need_strp_update || need_stmt_update;
+
+#if !_ELFUTILS_PREREQ (0, 169)
+  /* string replacements or build_id updates don't change section size. */
+  need_update = (need_update
+|| need_string_replacement
+|| (do_build_id && build_id != NULL));
+#endif
+
   /* We might have changed the size of some debug sections. If so make
  sure the section headers are updated and the data offsets are
  correct. We set ELF_F_LAYOUT above because we don't want libelf
@@ -2631,7 +2630,7 @@ main (int argc, char *argv[])
  anything for the phdrs allocated sections. Keep the offset of
  allocated sections so they are at the same place in the file. Add
  unallocated ones after the allocated ones. */
-  if (dso->phnum != 0 && (need_strp_update || need_stmt_update))
+  if (dso->phnum != 0 && need_update)
 {
   Elf *elf = dso->elf;
   GElf_Off last_offset;
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] build/files.c (processPackageFiles): Don't call generateBuildIDs for noarch.

[Mark Wielaard]

From: Mark Wielaard <m...@klomp.org>

We don't want to do build-id processing for noarch packages. It might be
that noarch packages do contain architecture depended files, but those are
already handled by processBinaryFiles.

This fixes the building of openbios in fedora.
https://bugzilla.redhat.com/show_bug.cgi?id=1433129

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 build/files.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/build/files.c b/build/files.c
index 6021643..35b2dd0 100644
--- a/build/files.c
+++ b/build/files.c
@@ -2384,10 +2384,14 @@ static rpmRC processPackageFiles(rpmSpec spec, 
rpmBuildPkgFlags pkgFlags,
goto exit;
 
 #if HAVE_LIBDW
-if (generateBuildIDs () != 0) {
-   rpmlog(RPMLOG_ERR, _("Generating build-id links failed\n"));
-   fl.processingFailed = 1;
-   goto exit;
+/* Check build-ids and add build-ids links for files to package list. */
+const char *arch = headerGetString(pkg->header, RPMTAG_ARCH);
+if (!rstreq(arch, "noarch")) {
+   if (generateBuildIDs () != 0) {
+   rpmlog(RPMLOG_ERR, _("Generating build-id links failed\n"));
+   fl.processingFailed = 1;
+   goto exit;
+   }
 }
 #endif
 
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] debugedit: Fix edit_dwarf2_line replace_dirs -> replace_files typo.

[Mark Wielaard]

From: Mark Wielaard <m...@klomp.org>

We wouldn't replace the changed file names if replace_dirs was false,
but replace_files was true. This could overrun the new debug_line data
buffer if the original file name was larger than the replacement. It
wasn't found before because often when we need to replace files we
also would have to replace dirs.

This fixes the kubernetes build in fedora.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 tools/debugedit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/debugedit.c b/tools/debugedit.c
index 4798c63..87a423f 100644
--- a/tools/debugedit.c
+++ b/tools/debugedit.c
@@ -1185,7 +1185,7 @@ edit_dwarf2_line (DSO *dso)
{
  const char *file = (const char *) optr;
  const char *file_path = NULL;
- if (t->replace_dirs)
+ if (t->replace_files)
{
  file_path = skip_dir_prefix (file, base_dir);
  if (file_path != NULL)
-- 
1.8.3.1

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] error: Missing build-id in /builddir/build/BUILDROOT/... (#177)

[Mark Wielaard]

On Wed, 2017-03-15 at 20:50 +0100, Mark Wielaard wrote:
> I couldn't replicate this with the current fedora guile-2.0.14-1
> package. There the .go files are not actual ELF files. So rpm will not
> complain about them not having a build-id. If that has changed in with
> guile 2.1.8 and the .go files are ELF binaries now and it is intended
> that they don't have a build-id then you could work around it by
> undefining %_missing_build_ids_terminate_build.
> 
> I'll try with the src.rpm you provided.

And indeed it seems guile switched to producing ELF files.
But note the following from
https://wingolog.org/archives/2014/01/19/elf-in-guile

Note that although Guile uses ELF on all platforms, we do not
use platform support for ELF. Guile implements its own linker
and loader. The advantage of using ELF is not sharing code, but
sharing ideas. ELF is simply a well-designed object file format.

So rpm is correct to warn that the ELF images don't contain build-ids.
As long as guile doesn't produce them you will need to add something
like the following to your spec file:

# Guile produces ELF images that are just containers for guile and don't
# include build-ids. 
https://wingolog.org/archives/2014/01/19/elf-in-guile
%undefine _missing_build_ids_terminate_build

That lets me rebuild your new guile srpm.
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint