All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/8] recordmcount cleanups
@ 2019-07-31 18:24 Matt Helsley
  2019-07-31 18:24 ` [PATCH v4 1/8] recordmcount: Remove redundant strcmp Matt Helsley
                   ` (8 more replies)
  0 siblings, 9 replies; 20+ messages in thread
From: Matt Helsley @ 2019-07-31 18:24 UTC (permalink / raw)
  To: LKML; +Cc: Ingo Molnar, Steven Rostedt, Matt Helsley

recordmcount presents unnecessary challenges to reviewers:

	It pretends to wrap access to the ELF file in
	uread/uwrite/ulseek functions which aren't related
	the way you might think (i.e. not the way read, write,
	and lseek are releated to each other).

	It uses setjmp/longjmp to handle errors (and success)
	during processing of the object files. This makes it
	hard to review what functions are doing, how globals
	change over time, etc.

	There are some kernel style nits.

This series addresses all of those by removing un-helper functions,
unused parameters, and rewriting the error/success handling to
better resemble regular kernel C code.

--

This series was formerly part of a v3 posted under the subject
"Cleanup recordmcount and begin objtool conversion". I am
reposting it split into two series: these cleanups of recordmcount
and a second series that begins the conversion of the cleaned-up
recordmcount into an objtool subcommand called mcount.

v4:
   Addressed feedback on v3 from Steven Rostedt:
       Moved already_has_mcount into recordmcount.c to avoid
                unnecessary multiple definitions.
       Changed return semantics of find_secsym_ndx() to avoid
                need for missing_sym (and multiple definitions)
                and to separate the returned symbol info
                (value, index) from success/failure indication.
       Fixed up local variable declaration to follow inverted
		christmas tree style.


Matt Helsley (8):
  recordmcount: Remove redundant strcmp
  recordmcount: Remove uread()
  recordmcount: Remove unused fd from uwrite() and ulseek()
  recordmcount: Rewrite error/success handling
  recordmcount: Kernel style function signature formatting
  recordmcount: Kernel style formatting
  recordmcount: Remove redundant cleanup() calls
  recordmcount: Clarify what cleanup() does

 scripts/recordmcount.c | 321 ++++++++++++++++++++---------------------
 scripts/recordmcount.h | 150 +++++++++++++------
 2 files changed, 259 insertions(+), 212 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 20+ messages in thread
* [PATCH v4 1/8] recordmcount: Remove redundant strcmp
  2019-07-31 18:24 [PATCH v4 0/8] recordmcount cleanups Matt Helsley
@ 2019-07-31 18:24 ` Matt Helsley
  2019-07-31 18:24 ` [PATCH v4 2/8] recordmcount: Remove uread() Matt Helsley
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Matt Helsley @ 2019-07-31 18:24 UTC (permalink / raw)
  To: LKML; +Cc: Ingo Molnar, Steven Rostedt, Matt Helsley

The strcmp is unnecessary since .text is already accepted as a
prefix in the strncmp().

Signed-off-by: Matt Helsley <mhelsley@vmware.com>
---
 scripts/recordmcount.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index 8387a9bc064a..ebe98c39f3cd 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -405,8 +405,7 @@ is_mcounted_section_name(char const *const txtname)
 		strcmp(".irqentry.text", txtname) == 0 ||
 		strcmp(".softirqentry.text", txtname) == 0 ||
 		strcmp(".kprobes.text", txtname) == 0 ||
-		strcmp(".cpuidle.text", txtname) == 0 ||
-		strcmp(".text.unlikely", txtname) == 0;
+		strcmp(".cpuidle.text", txtname) == 0;
 }
 
 /* 32 bit and 64 bit are very similar */
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v4 2/8] recordmcount: Remove uread()
  2019-07-31 18:24 [PATCH v4 0/8] recordmcount cleanups Matt Helsley
  2019-07-31 18:24 ` [PATCH v4 1/8] recordmcount: Remove redundant strcmp Matt Helsley
@ 2019-07-31 18:24 ` Matt Helsley
  2019-07-31 18:24 ` [PATCH v4 3/8] recordmcount: Remove unused fd from uwrite() and ulseek() Matt Helsley
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Matt Helsley @ 2019-07-31 18:24 UTC (permalink / raw)
  To: LKML; +Cc: Ingo Molnar, Steven Rostedt, Matt Helsley

uread() is only used to initialize the ELF file's pseudo
private-memory mapping while uwrite() and ulseek() work within
the pseudo-mapping and extend it as necessary.  Thus it is not
a complementary function to uwrite() and ulseek(). It also makes
no sense to do cleanups inside uread() when its only caller,
mmap_file(), is doing the relevant allocations and associated
initializations.

Therefore it's clearer to use a plain read() call to initialize the
data in mmap_file() and remove uread().

Signed-off-by: Matt Helsley <mhelsley@vmware.com>
---
 scripts/recordmcount.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index ebe98c39f3cd..c0dd46344063 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -89,7 +89,7 @@ succeed_file(void)
 	longjmp(jmpenv, SJ_SUCCEED);
 }
 
-/* ulseek, uread, ...:  Check return value for errors. */
+/* ulseek, uwrite, ...:  Check return value for errors. */
 
 static off_t
 ulseek(int const fd, off_t const offset, int const whence)
@@ -112,17 +112,6 @@ ulseek(int const fd, off_t const offset, int const whence)
 	return file_ptr - file_map;
 }
 
-static size_t
-uread(int const fd, void *const buf, size_t const count)
-{
-	size_t const n = read(fd, buf, count);
-	if (n != count) {
-		perror("read");
-		fail_file();
-	}
-	return n;
-}
-
 static size_t
 uwrite(int const fd, void const *const buf, size_t const count)
 {
@@ -298,7 +287,10 @@ static void *mmap_file(char const *fname)
 	if (file_map == MAP_FAILED) {
 		mmap_failed = 1;
 		file_map = umalloc(sb.st_size);
-		uread(fd_map, file_map, sb.st_size);
+		if (read(fd_map, file_map, sb.st_size) != sb.st_size) {
+			perror(fname);
+			fail_file();
+		}
 	}
 	close(fd_map);
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v4 3/8] recordmcount: Remove unused fd from uwrite() and ulseek()
  2019-07-31 18:24 [PATCH v4 0/8] recordmcount cleanups Matt Helsley
  2019-07-31 18:24 ` [PATCH v4 1/8] recordmcount: Remove redundant strcmp Matt Helsley
  2019-07-31 18:24 ` [PATCH v4 2/8] recordmcount: Remove uread() Matt Helsley
@ 2019-07-31 18:24 ` Matt Helsley
  2019-07-31 18:24 ` [PATCH v4 4/8] recordmcount: Rewrite error/success handling Matt Helsley
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Matt Helsley @ 2019-07-31 18:24 UTC (permalink / raw)
  To: LKML; +Cc: Ingo Molnar, Steven Rostedt, Matt Helsley

uwrite() works within the pseudo-mapping and extends it as necessary
without needing the file descriptor (fd) parameter passed to it.
Similarly, ulseek() doesn't need its fd parameter. These parameters
were only added because the functions bear a conceptual resemblance
to write() and lseek(). Worse, they obscure the fact that at the time
uwrite() and ulseek() are called fd_map is not a valid file descriptor.

Remove the unused file descriptor parameters that make it look like
fd_map is still valid.

Signed-off-by: Matt Helsley <mhelsley@vmware.com>
---
 scripts/recordmcount.c | 16 ++++++++--------
 scripts/recordmcount.h | 26 +++++++++++++-------------
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index c0dd46344063..1fe5fba99959 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -92,7 +92,7 @@ succeed_file(void)
 /* ulseek, uwrite, ...:  Check return value for errors. */
 
 static off_t
-ulseek(int const fd, off_t const offset, int const whence)
+ulseek(off_t const offset, int const whence)
 {
 	switch (whence) {
 	case SEEK_SET:
@@ -113,7 +113,7 @@ ulseek(int const fd, off_t const offset, int const whence)
 }
 
 static size_t
-uwrite(int const fd, void const *const buf, size_t const count)
+uwrite(void const *const buf, size_t const count)
 {
 	size_t cnt = count;
 	off_t idx = 0;
@@ -183,8 +183,8 @@ static int make_nop_x86(void *map, size_t const offset)
 		return -1;
 
 	/* convert to nop */
-	ulseek(fd_map, offset - 1, SEEK_SET);
-	uwrite(fd_map, ideal_nop, 5);
+	ulseek(offset - 1, SEEK_SET);
+	uwrite(ideal_nop, 5);
 	return 0;
 }
 
@@ -232,10 +232,10 @@ static int make_nop_arm(void *map, size_t const offset)
 		return -1;
 
 	/* Convert to nop */
-	ulseek(fd_map, off, SEEK_SET);
+	ulseek(off, SEEK_SET);
 
 	do {
-		uwrite(fd_map, ideal_nop, nop_size);
+		uwrite(ideal_nop, nop_size);
 	} while (--cnt > 0);
 
 	return 0;
@@ -252,8 +252,8 @@ static int make_nop_arm64(void *map, size_t const offset)
 		return -1;
 
 	/* Convert to nop */
-	ulseek(fd_map, offset, SEEK_SET);
-	uwrite(fd_map, ideal_nop, 4);
+	ulseek(offset, SEEK_SET);
+	uwrite(ideal_nop, 4);
 	return 0;
 }
 
diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index 47fca2c69a73..c1e1b04b4871 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -202,14 +202,14 @@ static void append_func(Elf_Ehdr *const ehdr,
 	new_e_shoff = t;
 
 	/* body for new shstrtab */
-	ulseek(fd_map, sb.st_size, SEEK_SET);
-	uwrite(fd_map, old_shstr_sh_offset + (void *)ehdr, old_shstr_sh_size);
-	uwrite(fd_map, mc_name, 1 + strlen(mc_name));
+	ulseek(sb.st_size, SEEK_SET);
+	uwrite(old_shstr_sh_offset + (void *)ehdr, old_shstr_sh_size);
+	uwrite(mc_name, 1 + strlen(mc_name));
 
 	/* old(modified) Elf_Shdr table, word-byte aligned */
-	ulseek(fd_map, t, SEEK_SET);
+	ulseek(t, SEEK_SET);
 	t += sizeof(Elf_Shdr) * old_shnum;
-	uwrite(fd_map, old_shoff + (void *)ehdr,
+	uwrite(old_shoff + (void *)ehdr,
 	       sizeof(Elf_Shdr) * old_shnum);
 
 	/* new sections __mcount_loc and .rel__mcount_loc */
@@ -225,7 +225,7 @@ static void append_func(Elf_Ehdr *const ehdr,
 	mcsec.sh_info = 0;
 	mcsec.sh_addralign = _w(_size);
 	mcsec.sh_entsize = _w(_size);
-	uwrite(fd_map, &mcsec, sizeof(mcsec));
+	uwrite(&mcsec, sizeof(mcsec));
 
 	mcsec.sh_name = w(old_shstr_sh_size);
 	mcsec.sh_type = (sizeof(Elf_Rela) == rel_entsize)
@@ -239,15 +239,15 @@ static void append_func(Elf_Ehdr *const ehdr,
 	mcsec.sh_info = w(old_shnum);
 	mcsec.sh_addralign = _w(_size);
 	mcsec.sh_entsize = _w(rel_entsize);
-	uwrite(fd_map, &mcsec, sizeof(mcsec));
+	uwrite(&mcsec, sizeof(mcsec));
 
-	uwrite(fd_map, mloc0, (void *)mlocp - (void *)mloc0);
-	uwrite(fd_map, mrel0, (void *)mrelp - (void *)mrel0);
+	uwrite(mloc0, (void *)mlocp - (void *)mloc0);
+	uwrite(mrel0, (void *)mrelp - (void *)mrel0);
 
 	ehdr->e_shoff = _w(new_e_shoff);
 	ehdr->e_shnum = w2(2 + w2(ehdr->e_shnum));  /* {.rel,}__mcount_loc */
-	ulseek(fd_map, 0, SEEK_SET);
-	uwrite(fd_map, ehdr, sizeof(*ehdr));
+	ulseek(0, SEEK_SET);
+	uwrite(ehdr, sizeof(*ehdr));
 }
 
 static unsigned get_mcountsym(Elf_Sym const *const sym0,
@@ -396,8 +396,8 @@ static void nop_mcount(Elf_Shdr const *const relhdr,
 			Elf_Rel rel;
 			rel = *(Elf_Rel *)relp;
 			Elf_r_info(&rel, Elf_r_sym(relp), rel_type_nop);
-			ulseek(fd_map, (void *)relp - (void *)ehdr, SEEK_SET);
-			uwrite(fd_map, &rel, sizeof(rel));
+			ulseek((void *)relp - (void *)ehdr, SEEK_SET);
+			uwrite(&rel, sizeof(rel));
 		}
 		relp = (Elf_Rel const *)(rel_entsize + (void *)relp);
 	}
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v4 4/8] recordmcount: Rewrite error/success handling
  2019-07-31 18:24 [PATCH v4 0/8] recordmcount cleanups Matt Helsley
                   ` (2 preceding siblings ...)
  2019-07-31 18:24 ` [PATCH v4 3/8] recordmcount: Remove unused fd from uwrite() and ulseek() Matt Helsley
@ 2019-07-31 18:24 ` Matt Helsley
  2019-10-09 10:46     ` Uwe Kleine-König
  2019-07-31 18:24 ` [PATCH v4 5/8] recordmcount: Kernel style function signature formatting Matt Helsley
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Matt Helsley @ 2019-07-31 18:24 UTC (permalink / raw)
  To: LKML; +Cc: Ingo Molnar, Steven Rostedt, Matt Helsley

Recordmcount uses setjmp/longjmp to manage control flow as
it reads and then writes the ELF file. This unusual control
flow is hard to follow and check in addition to being unlike
kernel coding style.

So we rewrite these paths to use regular return values to
indicate error/success. When an error or previously-completed object
file is found we return an error code following kernel
coding conventions -- negative error values and 0 for success when
we're not returning a pointer. We return NULL for those that fail
and return non-NULL pointers otherwise.

One oddity is already_has_rel_mcount -- there we use pointer comparison
rather than string comparison to differentiate between
previously-processed object files and returning the name of a text
section.

Signed-off-by: Matt Helsley <mhelsley@vmware.com>

--

v3.5 -- Moved already_has_mcount into recordmcount.c to avoid
		unnecessary multiple definitions
	Changed return semantics of find_secsym_ndx() to avoid
		need for missing_sym (and multiple definitions)
		and to separate the returned symbol info
		(value, index) from success/failure indication
---
 scripts/recordmcount.c | 162 +++++++++++++++++++++--------------------
 scripts/recordmcount.h | 141 ++++++++++++++++++++++++-----------
 2 files changed, 184 insertions(+), 119 deletions(-)

diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index 1fe5fba99959..c6d395b8ff29 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -27,7 +27,6 @@
 #include <getopt.h>
 #include <elf.h>
 #include <fcntl.h>
-#include <setjmp.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -43,7 +42,6 @@ static int fd_map;	/* File descriptor for file being modified. */
 static int mmap_failed; /* Boolean flag. */
 static char gpfx;	/* prefix for global symbol name (sometimes '_') */
 static struct stat sb;	/* Remember .st_size, etc. */
-static jmp_buf jmpenv;	/* setjmp/longjmp per-file error escape */
 static const char *altmcount;	/* alternate mcount symbol name */
 static int warn_on_notrace_sect; /* warn when section has mcount not being recorded */
 static void *file_map;	/* pointer of the mapped file */
@@ -53,13 +51,6 @@ static void *file_ptr;	/* current file pointer location */
 static void *file_append; /* added to the end of the file */
 static size_t file_append_size; /* how much is added to end of file */
 
-/* setjmp() return values */
-enum {
-	SJ_SETJMP = 0,  /* hardwired first return */
-	SJ_FAIL,
-	SJ_SUCCEED
-};
-
 /* Per-file resource cleanup when multiple files. */
 static void
 cleanup(void)
@@ -75,20 +66,6 @@ cleanup(void)
 	file_updated = 0;
 }
 
-static void __attribute__((noreturn))
-fail_file(void)
-{
-	cleanup();
-	longjmp(jmpenv, SJ_FAIL);
-}
-
-static void __attribute__((noreturn))
-succeed_file(void)
-{
-	cleanup();
-	longjmp(jmpenv, SJ_SUCCEED);
-}
-
 /* ulseek, uwrite, ...:  Check return value for errors. */
 
 static off_t
@@ -107,12 +84,12 @@ ulseek(off_t const offset, int const whence)
 	}
 	if (file_ptr < file_map) {
 		fprintf(stderr, "lseek: seek before file\n");
-		fail_file();
+		return -1;
 	}
 	return file_ptr - file_map;
 }
 
-static size_t
+static ssize_t
 uwrite(void const *const buf, size_t const count)
 {
 	size_t cnt = count;
@@ -129,7 +106,8 @@ uwrite(void const *const buf, size_t const count)
 		}
 		if (!file_append) {
 			perror("write");
-			fail_file();
+			cleanup();
+			return -1;
 		}
 		if (file_ptr < file_end) {
 			cnt = file_end - file_ptr;
@@ -155,7 +133,8 @@ umalloc(size_t size)
 	void *const addr = malloc(size);
 	if (addr == 0) {
 		fprintf(stderr, "malloc failed: %zu bytes\n", size);
-		fail_file();
+		cleanup();
+		return NULL;
 	}
 	return addr;
 }
@@ -183,8 +162,10 @@ static int make_nop_x86(void *map, size_t const offset)
 		return -1;
 
 	/* convert to nop */
-	ulseek(offset - 1, SEEK_SET);
-	uwrite(ideal_nop, 5);
+	if (ulseek(offset - 1, SEEK_SET) < 0)
+		return -1;
+	if (uwrite(ideal_nop, 5) < 0)
+		return -1;
 	return 0;
 }
 
@@ -232,10 +213,12 @@ static int make_nop_arm(void *map, size_t const offset)
 		return -1;
 
 	/* Convert to nop */
-	ulseek(off, SEEK_SET);
+	if (ulseek(off, SEEK_SET) < 0)
+		return -1;
 
 	do {
-		uwrite(ideal_nop, nop_size);
+		if (uwrite(ideal_nop, nop_size) < 0)
+			return -1;
 	} while (--cnt > 0);
 
 	return 0;
@@ -252,8 +235,10 @@ static int make_nop_arm64(void *map, size_t const offset)
 		return -1;
 
 	/* Convert to nop */
-	ulseek(offset, SEEK_SET);
-	uwrite(ideal_nop, 4);
+	if (ulseek(offset, SEEK_SET) < 0)
+		return -1;
+	if (uwrite(ideal_nop, 4) < 0)
+		return -1;
 	return 0;
 }
 
@@ -272,14 +257,23 @@ static int make_nop_arm64(void *map, size_t const offset)
  */
 static void *mmap_file(char const *fname)
 {
+	file_map = NULL;
+	sb.st_size = 0;
 	fd_map = open(fname, O_RDONLY);
-	if (fd_map < 0 || fstat(fd_map, &sb) < 0) {
+	if (fd_map < 0) {
+		perror(fname);
+		cleanup();
+		return NULL;
+	}
+	if (fstat(fd_map, &sb) < 0) {
 		perror(fname);
-		fail_file();
+		cleanup();
+		goto out;
 	}
 	if (!S_ISREG(sb.st_mode)) {
 		fprintf(stderr, "not a regular file: %s\n", fname);
-		fail_file();
+		cleanup();
+		goto out;
 	}
 	file_map = mmap(0, sb.st_size, PROT_READ|PROT_WRITE, MAP_PRIVATE,
 			fd_map, 0);
@@ -287,11 +281,18 @@ static void *mmap_file(char const *fname)
 	if (file_map == MAP_FAILED) {
 		mmap_failed = 1;
 		file_map = umalloc(sb.st_size);
+		if (!file_map) {
+			perror(fname);
+			goto out;
+		}
 		if (read(fd_map, file_map, sb.st_size) != sb.st_size) {
 			perror(fname);
-			fail_file();
+			free(file_map);
+			file_map = NULL;
+			goto out;
 		}
 	}
+out:
 	close(fd_map);
 
 	file_end = file_map + sb.st_size;
@@ -299,13 +300,13 @@ static void *mmap_file(char const *fname)
 	return file_map;
 }
 
-static void write_file(const char *fname)
+static int write_file(const char *fname)
 {
 	char tmp_file[strlen(fname) + 4];
 	size_t n;
 
 	if (!file_updated)
-		return;
+		return 0;
 
 	sprintf(tmp_file, "%s.rc", fname);
 
@@ -317,25 +318,32 @@ static void write_file(const char *fname)
 	fd_map = open(tmp_file, O_WRONLY | O_TRUNC | O_CREAT, sb.st_mode);
 	if (fd_map < 0) {
 		perror(fname);
-		fail_file();
+		cleanup();
+		return -1;
 	}
 	n = write(fd_map, file_map, sb.st_size);
 	if (n != sb.st_size) {
 		perror("write");
-		fail_file();
+		cleanup();
+		close(fd_map);
+		return -1;
 	}
 	if (file_append_size) {
 		n = write(fd_map, file_append, file_append_size);
 		if (n != file_append_size) {
 			perror("write");
-			fail_file();
+			cleanup();
+			close(fd_map);
+			return -1;
 		}
 	}
 	close(fd_map);
 	if (rename(tmp_file, fname) < 0) {
 		perror(fname);
-		fail_file();
+		cleanup();
+		return -1;
 	}
+	return 0;
 }
 
 /* w8rev, w8nat, ...: Handle endianness. */
@@ -400,6 +408,8 @@ is_mcounted_section_name(char const *const txtname)
 		strcmp(".cpuidle.text", txtname) == 0;
 }
 
+static char const *already_has_rel_mcount = "success"; /* our work here is done! */
+
 /* 32 bit and 64 bit are very similar */
 #include "recordmcount.h"
 #define RECORD_MCOUNT_64
@@ -438,11 +448,15 @@ static void MIPS64_r_info(Elf64_Rel *const rp, unsigned sym, unsigned type)
 	}).r_info;
 }
 
-static void
+static int
 do_file(char const *const fname)
 {
 	Elf32_Ehdr *const ehdr = mmap_file(fname);
 	unsigned int reltype = 0;
+	int rc = -1;
+
+	if (!ehdr)
+		goto out;
 
 	w = w4nat;
 	w2 = w2nat;
@@ -452,8 +466,8 @@ do_file(char const *const fname)
 	default:
 		fprintf(stderr, "unrecognized ELF data encoding %d: %s\n",
 			ehdr->e_ident[EI_DATA], fname);
-		fail_file();
-		break;
+		cleanup();
+		goto out;
 	case ELFDATA2LSB:
 		if (*(unsigned char const *)&endian != 1) {
 			/* main() is big endian, file.o is little endian. */
@@ -485,7 +499,8 @@ do_file(char const *const fname)
 	||  w2(ehdr->e_type) != ET_REL
 	||  ehdr->e_ident[EI_VERSION] != EV_CURRENT) {
 		fprintf(stderr, "unrecognized ET_REL file %s\n", fname);
-		fail_file();
+		cleanup();
+		goto out;
 	}
 
 	gpfx = 0;
@@ -493,8 +508,8 @@ do_file(char const *const fname)
 	default:
 		fprintf(stderr, "unrecognized e_machine %u %s\n",
 			w2(ehdr->e_machine), fname);
-		fail_file();
-		break;
+		cleanup();
+		goto out;
 	case EM_386:
 		reltype = R_386_32;
 		rel_type_nop = R_386_NONE;
@@ -534,20 +549,22 @@ do_file(char const *const fname)
 	default:
 		fprintf(stderr, "unrecognized ELF class %d %s\n",
 			ehdr->e_ident[EI_CLASS], fname);
-		fail_file();
-		break;
+		cleanup();
+		goto out;
 	case ELFCLASS32:
 		if (w2(ehdr->e_ehsize) != sizeof(Elf32_Ehdr)
 		||  w2(ehdr->e_shentsize) != sizeof(Elf32_Shdr)) {
 			fprintf(stderr,
 				"unrecognized ET_REL file: %s\n", fname);
-			fail_file();
+			cleanup();
+			goto out;
 		}
 		if (w2(ehdr->e_machine) == EM_MIPS) {
 			reltype = R_MIPS_32;
 			is_fake_mcount32 = MIPS32_is_fake_mcount;
 		}
-		do32(ehdr, fname, reltype);
+		if (do32(ehdr, fname, reltype) < 0)
+			goto out;
 		break;
 	case ELFCLASS64: {
 		Elf64_Ehdr *const ghdr = (Elf64_Ehdr *)ehdr;
@@ -555,7 +572,8 @@ do_file(char const *const fname)
 		||  w2(ghdr->e_shentsize) != sizeof(Elf64_Shdr)) {
 			fprintf(stderr,
 				"unrecognized ET_REL file: %s\n", fname);
-			fail_file();
+			cleanup();
+			goto out;
 		}
 		if (w2(ghdr->e_machine) == EM_S390) {
 			reltype = R_390_64;
@@ -567,13 +585,16 @@ do_file(char const *const fname)
 			Elf64_r_info = MIPS64_r_info;
 			is_fake_mcount64 = MIPS64_is_fake_mcount;
 		}
-		do64(ghdr, fname, reltype);
+		if (do64(ghdr, fname, reltype) < 0)
+			goto out;
 		break;
 	}
 	}  /* end switch */
 
-	write_file(fname);
+	rc = write_file(fname);
+out:
 	cleanup();
+	return rc;
 }
 
 int
@@ -604,7 +625,6 @@ main(int argc, char *argv[])
 	/* Process each file in turn, allowing deep failure. */
 	for (i = optind; i < argc; i++) {
 		char *file = argv[i];
-		int const sjval = setjmp(jmpenv);
 		int len;
 
 		/*
@@ -617,28 +637,16 @@ main(int argc, char *argv[])
 		    strcmp(file + (len - ftrace_size), ftrace) == 0)
 			continue;
 
-		switch (sjval) {
-		default:
-			fprintf(stderr, "internal error: %s\n", file);
-			exit(1);
-			break;
-		case SJ_SETJMP:    /* normal sequence */
-			/* Avoid problems if early cleanup() */
-			fd_map = -1;
-			mmap_failed = 1;
-			file_map = NULL;
-			file_ptr = NULL;
-			file_updated = 0;
-			do_file(file);
-			break;
-		case SJ_FAIL:    /* error in do_file or below */
+		/* Avoid problems if early cleanup() */
+		fd_map = -1;
+		mmap_failed = 1;
+		file_map = NULL;
+		file_ptr = NULL;
+		file_updated = 0;
+		if (do_file(file)) {
 			fprintf(stderr, "%s: failed\n", file);
 			++n_error;
-			break;
-		case SJ_SUCCEED:    /* premature success */
-			/* do nothing */
-			break;
-		}  /* end switch */
+		}
 	}
 	return !!n_error;
 }
diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index c1e1b04b4871..3796eb37fb12 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -174,7 +174,7 @@ static int MIPS_is_fake_mcount(Elf_Rel const *rp)
 }
 
 /* Append the new shstrtab, Elf_Shdr[], __mcount_loc and its relocations. */
-static void append_func(Elf_Ehdr *const ehdr,
+static int append_func(Elf_Ehdr *const ehdr,
 			Elf_Shdr *const shstr,
 			uint_t const *const mloc0,
 			uint_t const *const mlocp,
@@ -202,15 +202,20 @@ static void append_func(Elf_Ehdr *const ehdr,
 	new_e_shoff = t;
 
 	/* body for new shstrtab */
-	ulseek(sb.st_size, SEEK_SET);
-	uwrite(old_shstr_sh_offset + (void *)ehdr, old_shstr_sh_size);
-	uwrite(mc_name, 1 + strlen(mc_name));
+	if (ulseek(sb.st_size, SEEK_SET) < 0)
+		return -1;
+	if (uwrite(old_shstr_sh_offset + (void *)ehdr, old_shstr_sh_size) < 0)
+		return -1;
+	if (uwrite(mc_name, 1 + strlen(mc_name)) < 0)
+		return -1;
 
 	/* old(modified) Elf_Shdr table, word-byte aligned */
-	ulseek(t, SEEK_SET);
+	if (ulseek(t, SEEK_SET) < 0)
+		return -1;
 	t += sizeof(Elf_Shdr) * old_shnum;
-	uwrite(old_shoff + (void *)ehdr,
-	       sizeof(Elf_Shdr) * old_shnum);
+	if (uwrite(old_shoff + (void *)ehdr,
+	       sizeof(Elf_Shdr) * old_shnum) < 0)
+		return -1;
 
 	/* new sections __mcount_loc and .rel__mcount_loc */
 	t += 2*sizeof(mcsec);
@@ -225,7 +230,8 @@ static void append_func(Elf_Ehdr *const ehdr,
 	mcsec.sh_info = 0;
 	mcsec.sh_addralign = _w(_size);
 	mcsec.sh_entsize = _w(_size);
-	uwrite(&mcsec, sizeof(mcsec));
+	if (uwrite(&mcsec, sizeof(mcsec)) < 0)
+		return -1;
 
 	mcsec.sh_name = w(old_shstr_sh_size);
 	mcsec.sh_type = (sizeof(Elf_Rela) == rel_entsize)
@@ -239,15 +245,22 @@ static void append_func(Elf_Ehdr *const ehdr,
 	mcsec.sh_info = w(old_shnum);
 	mcsec.sh_addralign = _w(_size);
 	mcsec.sh_entsize = _w(rel_entsize);
-	uwrite(&mcsec, sizeof(mcsec));
 
-	uwrite(mloc0, (void *)mlocp - (void *)mloc0);
-	uwrite(mrel0, (void *)mrelp - (void *)mrel0);
+	if (uwrite(&mcsec, sizeof(mcsec)) < 0)
+		return -1;
+
+	if (uwrite(mloc0, (void *)mlocp - (void *)mloc0) < 0)
+		return -1;
+	if (uwrite(mrel0, (void *)mrelp - (void *)mrel0) < 0)
+		return -1;
 
 	ehdr->e_shoff = _w(new_e_shoff);
 	ehdr->e_shnum = w2(2 + w2(ehdr->e_shnum));  /* {.rel,}__mcount_loc */
-	ulseek(0, SEEK_SET);
-	uwrite(ehdr, sizeof(*ehdr));
+	if (ulseek(0, SEEK_SET) < 0)
+		return -1;
+	if (uwrite(ehdr, sizeof(*ehdr)) < 0)
+		return -1;
+	return 0;
 }
 
 static unsigned get_mcountsym(Elf_Sym const *const sym0,
@@ -351,9 +364,9 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
  * that are not going to be traced. The mcount calls here will be converted
  * into nops.
  */
-static void nop_mcount(Elf_Shdr const *const relhdr,
-		       Elf_Ehdr const *const ehdr,
-		       const char *const txtname)
+static int nop_mcount(Elf_Shdr const *const relhdr,
+		      Elf_Ehdr const *const ehdr,
+		      const char *const txtname)
 {
 	Elf_Shdr *const shdr0 = (Elf_Shdr *)(_w(ehdr->e_shoff)
 		+ (void *)ehdr);
@@ -376,15 +389,18 @@ static void nop_mcount(Elf_Shdr const *const relhdr,
 			mcountsym = get_mcountsym(sym0, relp, str0);
 
 		if (mcountsym == Elf_r_sym(relp) && !is_fake_mcount(relp)) {
-			if (make_nop)
+			if (make_nop) {
 				ret = make_nop((void *)ehdr, _w(shdr->sh_offset) + _w(relp->r_offset));
+				if (ret < 0)
+					return -1;
+			}
 			if (warn_on_notrace_sect && !once) {
 				printf("Section %s has mcount callers being ignored\n",
 				       txtname);
 				once = 1;
 				/* just warn? */
 				if (!make_nop)
-					return;
+					return 0;
 			}
 		}
 
@@ -396,14 +412,16 @@ static void nop_mcount(Elf_Shdr const *const relhdr,
 			Elf_Rel rel;
 			rel = *(Elf_Rel *)relp;
 			Elf_r_info(&rel, Elf_r_sym(relp), rel_type_nop);
-			ulseek((void *)relp - (void *)ehdr, SEEK_SET);
-			uwrite(&rel, sizeof(rel));
+			if (ulseek((void *)relp - (void *)ehdr, SEEK_SET) < 0)
+				return -1;
+			if (uwrite(&rel, sizeof(rel)) < 0)
+				return -1;
 		}
 		relp = (Elf_Rel const *)(rel_entsize + (void *)relp);
 	}
+	return 0;
 }
 
-
 /*
  * Find a symbol in the given section, to be used as the base for relocating
  * the table of offsets of calls to mcount.  A local or global symbol suffices,
@@ -414,9 +432,10 @@ static void nop_mcount(Elf_Shdr const *const relhdr,
  *    Num:    Value  Size Type    Bind   Vis      Ndx Name
  *      2: 00000000     0 SECTION LOCAL  DEFAULT    1
  */
-static unsigned find_secsym_ndx(unsigned const txtndx,
+static int find_secsym_ndx(unsigned const txtndx,
 				char const *const txtname,
 				uint_t *const recvalp,
+				unsigned int *sym_index,
 				Elf_Shdr const *const symhdr,
 				Elf_Ehdr const *const ehdr)
 {
@@ -438,15 +457,16 @@ static unsigned find_secsym_ndx(unsigned const txtndx,
 				continue;
 
 			*recvalp = _w(symp->st_value);
-			return symp - sym0;
+			*sym_index = symp - sym0;
+			return 0;
 		}
 	}
 	fprintf(stderr, "Cannot find symbol for section %u: %s.\n",
 		txtndx, txtname);
-	fail_file();
+	cleanup();
+	return -1;
 }
 
-
 /* Evade ISO C restriction: no declaration after statement in has_rel_mcount. */
 static char const *
 __has_rel_mcount(Elf_Shdr const *const relhdr,  /* is SHT_REL or SHT_RELA */
@@ -461,7 +481,8 @@ __has_rel_mcount(Elf_Shdr const *const relhdr,  /* is SHT_REL or SHT_RELA */
 	if (strcmp("__mcount_loc", txtname) == 0) {
 		fprintf(stderr, "warning: __mcount_loc already exists: %s\n",
 			fname);
-		succeed_file();
+		cleanup();
+		return already_has_rel_mcount;
 	}
 	if (w(txthdr->sh_type) != SHT_PROGBITS ||
 	    !(_w(txthdr->sh_flags) & SHF_EXECINSTR))
@@ -491,6 +512,10 @@ static unsigned tot_relsize(Elf_Shdr const *const shdr0,
 
 	for (; nhdr; --nhdr, ++shdrp) {
 		txtname = has_rel_mcount(shdrp, shdr0, shstrtab, fname);
+		if (txtname == already_has_rel_mcount) {
+			totrelsz = 0;
+			break;
+		}
 		if (txtname && is_mcounted_section_name(txtname))
 			totrelsz += _w(shdrp->sh_size);
 	}
@@ -499,7 +524,7 @@ static unsigned tot_relsize(Elf_Shdr const *const shdr0,
 
 
 /* Overall supervision for Elf32 ET_REL file. */
-static void
+static int
 do_func(Elf_Ehdr *const ehdr, char const *const fname, unsigned const reltype)
 {
 	Elf_Shdr *const shdr0 = (Elf_Shdr *)(_w(ehdr->e_shoff)
@@ -513,26 +538,54 @@ do_func(Elf_Ehdr *const ehdr, char const *const fname, unsigned const reltype)
 	unsigned k;
 
 	/* Upper bound on space: assume all relevant relocs are for mcount. */
-	unsigned const totrelsz = tot_relsize(shdr0, nhdr, shstrtab, fname);
-	Elf_Rel *const mrel0 = umalloc(totrelsz);
-	Elf_Rel *      mrelp = mrel0;
+	unsigned       totrelsz;
 
-	/* 2*sizeof(address) <= sizeof(Elf_Rel) */
-	uint_t *const mloc0 = umalloc(totrelsz>>1);
-	uint_t *      mlocp = mloc0;
+	Elf_Rel *      mrel0;
+	Elf_Rel *      mrelp;
+
+	uint_t *      mloc0;
+	uint_t *      mlocp;
 
 	unsigned rel_entsize = 0;
 	unsigned symsec_sh_link = 0;
 
+	int result = 0;
+
+	totrelsz = tot_relsize(shdr0, nhdr, shstrtab, fname);
+	if (totrelsz == 0)
+		return 0;
+	mrel0 = umalloc(totrelsz);
+	mrelp = mrel0;
+	if (!mrel0)
+		return -1;
+
+	/* 2*sizeof(address) <= sizeof(Elf_Rel) */
+	mloc0 = umalloc(totrelsz>>1);
+	mlocp = mloc0;
+	if (!mloc0) {
+		free(mrel0);
+		return -1;
+	}
+
 	for (relhdr = shdr0, k = nhdr; k; --k, ++relhdr) {
 		char const *const txtname = has_rel_mcount(relhdr, shdr0,
 			shstrtab, fname);
+		if (txtname == already_has_rel_mcount) {
+			result = 0;
+			file_updated = 0;
+			goto out; /* Nothing to be done; don't append! */
+		}
 		if (txtname && is_mcounted_section_name(txtname)) {
+			unsigned int recsym;
 			uint_t recval = 0;
-			unsigned const recsym = find_secsym_ndx(
-				w(relhdr->sh_info), txtname, &recval,
-				&shdr0[symsec_sh_link = w(relhdr->sh_link)],
-				ehdr);
+
+			symsec_sh_link = w(relhdr->sh_link);
+			result = find_secsym_ndx(w(relhdr->sh_info), txtname,
+						&recval, &recsym,
+						&shdr0[symsec_sh_link],
+						ehdr);
+			if (result)
+				goto out;
 
 			rel_entsize = _w(relhdr->sh_entsize);
 			mlocp = sift_rel_mcount(mlocp,
@@ -543,13 +596,17 @@ do_func(Elf_Ehdr *const ehdr, char const *const fname, unsigned const reltype)
 			 * This section is ignored by ftrace, but still
 			 * has mcount calls. Convert them to nops now.
 			 */
-			nop_mcount(relhdr, ehdr, txtname);
+			if (nop_mcount(relhdr, ehdr, txtname) < 0) {
+				result = -1;
+				goto out;
+			}
 		}
 	}
-	if (mloc0 != mlocp) {
-		append_func(ehdr, shstr, mloc0, mlocp, mrel0, mrelp,
-			    rel_entsize, symsec_sh_link);
-	}
+	if (!result && mloc0 != mlocp)
+		result = append_func(ehdr, shstr, mloc0, mlocp, mrel0, mrelp,
+				     rel_entsize, symsec_sh_link);
+out:
 	free(mrel0);
 	free(mloc0);
+	return result;
 }
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v4 5/8] recordmcount: Kernel style function signature formatting
  2019-07-31 18:24 [PATCH v4 0/8] recordmcount cleanups Matt Helsley
                   ` (3 preceding siblings ...)
  2019-07-31 18:24 ` [PATCH v4 4/8] recordmcount: Rewrite error/success handling Matt Helsley
@ 2019-07-31 18:24 ` Matt Helsley
  2019-07-31 18:24 ` [PATCH v4 6/8] recordmcount: Kernel style formatting Matt Helsley
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Matt Helsley @ 2019-07-31 18:24 UTC (permalink / raw)
  To: LKML; +Cc: Ingo Molnar, Steven Rostedt, Matt Helsley

The uwrite() and ulseek() functions are formatted inconsistently
with the rest of the file and the kernel overall. While we're
making other changes here let's fix this.

Signed-off-by: Matt Helsley <mhelsley@vmware.com>
---
 scripts/recordmcount.c | 21 +++++++--------------
 scripts/recordmcount.h | 13 ++++++-------
 2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index c6d395b8ff29..67f9c45b824f 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -52,8 +52,7 @@ static void *file_append; /* added to the end of the file */
 static size_t file_append_size; /* how much is added to end of file */
 
 /* Per-file resource cleanup when multiple files. */
-static void
-cleanup(void)
+static void cleanup(void)
 {
 	if (!mmap_failed)
 		munmap(file_map, sb.st_size);
@@ -68,8 +67,7 @@ cleanup(void)
 
 /* ulseek, uwrite, ...:  Check return value for errors. */
 
-static off_t
-ulseek(off_t const offset, int const whence)
+static off_t ulseek(off_t const offset, int const whence)
 {
 	switch (whence) {
 	case SEEK_SET:
@@ -89,8 +87,7 @@ ulseek(off_t const offset, int const whence)
 	return file_ptr - file_map;
 }
 
-static ssize_t
-uwrite(void const *const buf, size_t const count)
+static ssize_t uwrite(void const *const buf, size_t const count)
 {
 	size_t cnt = count;
 	off_t idx = 0;
@@ -127,8 +124,7 @@ uwrite(void const *const buf, size_t const count)
 	return count;
 }
 
-static void *
-umalloc(size_t size)
+static void * umalloc(size_t size)
 {
 	void *const addr = malloc(size);
 	if (addr == 0) {
@@ -394,8 +390,7 @@ static uint32_t (*w)(uint32_t);
 static uint32_t (*w2)(uint16_t);
 
 /* Names of the sections that could contain calls to mcount. */
-static int
-is_mcounted_section_name(char const *const txtname)
+static int is_mcounted_section_name(char const *const txtname)
 {
 	return strncmp(".text",          txtname, 5) == 0 ||
 		strcmp(".init.text",     txtname) == 0 ||
@@ -448,8 +443,7 @@ static void MIPS64_r_info(Elf64_Rel *const rp, unsigned sym, unsigned type)
 	}).r_info;
 }
 
-static int
-do_file(char const *const fname)
+static int do_file(char const *const fname)
 {
 	Elf32_Ehdr *const ehdr = mmap_file(fname);
 	unsigned int reltype = 0;
@@ -597,8 +591,7 @@ do_file(char const *const fname)
 	return rc;
 }
 
-int
-main(int argc, char *argv[])
+int main(int argc, char *argv[])
 {
 	const char ftrace[] = "/ftrace.o";
 	int ftrace_size = sizeof(ftrace) - 1;
diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index 3796eb37fb12..ca9aaac89bfb 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -468,11 +468,10 @@ static int find_secsym_ndx(unsigned const txtndx,
 }
 
 /* Evade ISO C restriction: no declaration after statement in has_rel_mcount. */
-static char const *
-__has_rel_mcount(Elf_Shdr const *const relhdr,  /* is SHT_REL or SHT_RELA */
-		 Elf_Shdr const *const shdr0,
-		 char const *const shstrtab,
-		 char const *const fname)
+static char const * __has_rel_mcount(Elf_Shdr const *const relhdr, /* reltype */
+				     Elf_Shdr const *const shdr0,
+				     char const *const shstrtab,
+				     char const *const fname)
 {
 	/* .sh_info depends on .sh_type == SHT_REL[,A] */
 	Elf_Shdr const *const txthdr = &shdr0[w(relhdr->sh_info)];
@@ -524,8 +523,8 @@ static unsigned tot_relsize(Elf_Shdr const *const shdr0,
 
 
 /* Overall supervision for Elf32 ET_REL file. */
-static int
-do_func(Elf_Ehdr *const ehdr, char const *const fname, unsigned const reltype)
+static int do_func(Elf_Ehdr *const ehdr, char const *const fname,
+		   unsigned const reltype)
 {
 	Elf_Shdr *const shdr0 = (Elf_Shdr *)(_w(ehdr->e_shoff)
 		+ (void *)ehdr);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v4 6/8] recordmcount: Kernel style formatting
  2019-07-31 18:24 [PATCH v4 0/8] recordmcount cleanups Matt Helsley
                   ` (4 preceding siblings ...)
  2019-07-31 18:24 ` [PATCH v4 5/8] recordmcount: Kernel style function signature formatting Matt Helsley
@ 2019-07-31 18:24 ` Matt Helsley
  2019-07-31 18:24 ` [PATCH v4 7/8] recordmcount: Remove redundant cleanup() calls Matt Helsley
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Matt Helsley @ 2019-07-31 18:24 UTC (permalink / raw)
  To: LKML; +Cc: Ingo Molnar, Steven Rostedt, Matt Helsley

Fix up the whitespace irregularity in the ELF switch
blocks.

Swapping the initial value of gpfx allows us to
simplify all but one of the one-line switch cases even
further.

Signed-off-by: Matt Helsley <mhelsley@vmware.com>
---
 scripts/recordmcount.c | 47 ++++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 22 deletions(-)

diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index 67f9c45b824f..273ca8b42b20 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -489,15 +489,15 @@ static int do_file(char const *const fname)
 		push_bl_mcount_thumb = push_bl_mcount_thumb_be;
 		break;
 	}  /* end switch */
-	if (memcmp(ELFMAG, ehdr->e_ident, SELFMAG) != 0
-	||  w2(ehdr->e_type) != ET_REL
-	||  ehdr->e_ident[EI_VERSION] != EV_CURRENT) {
+	if (memcmp(ELFMAG, ehdr->e_ident, SELFMAG) != 0 ||
+	    w2(ehdr->e_type) != ET_REL ||
+	    ehdr->e_ident[EI_VERSION] != EV_CURRENT) {
 		fprintf(stderr, "unrecognized ET_REL file %s\n", fname);
 		cleanup();
 		goto out;
 	}
 
-	gpfx = 0;
+	gpfx = '_';
 	switch (w2(ehdr->e_machine)) {
 	default:
 		fprintf(stderr, "unrecognized e_machine %u %s\n",
@@ -510,32 +510,35 @@ static int do_file(char const *const fname)
 		make_nop = make_nop_x86;
 		ideal_nop = ideal_nop5_x86_32;
 		mcount_adjust_32 = -1;
+		gpfx = 0;
+		break;
+	case EM_ARM:
+		reltype = R_ARM_ABS32;
+		altmcount = "__gnu_mcount_nc";
+		make_nop = make_nop_arm;
+		rel_type_nop = R_ARM_NONE;
+		gpfx = 0;
 		break;
-	case EM_ARM:	 reltype = R_ARM_ABS32;
-			 altmcount = "__gnu_mcount_nc";
-			 make_nop = make_nop_arm;
-			 rel_type_nop = R_ARM_NONE;
-			 break;
 	case EM_AARCH64:
-			reltype = R_AARCH64_ABS64;
-			make_nop = make_nop_arm64;
-			rel_type_nop = R_AARCH64_NONE;
-			ideal_nop = ideal_nop4_arm64;
-			gpfx = '_';
-			break;
-	case EM_IA_64:	 reltype = R_IA64_IMM64;   gpfx = '_'; break;
-	case EM_MIPS:	 /* reltype: e_class    */ gpfx = '_'; break;
-	case EM_PPC:	 reltype = R_PPC_ADDR32;   gpfx = '_'; break;
-	case EM_PPC64:	 reltype = R_PPC64_ADDR64; gpfx = '_'; break;
-	case EM_S390:    /* reltype: e_class    */ gpfx = '_'; break;
-	case EM_SH:	 reltype = R_SH_DIR32;                 break;
-	case EM_SPARCV9: reltype = R_SPARC_64;     gpfx = '_'; break;
+		reltype = R_AARCH64_ABS64;
+		make_nop = make_nop_arm64;
+		rel_type_nop = R_AARCH64_NONE;
+		ideal_nop = ideal_nop4_arm64;
+		break;
+	case EM_IA_64:	reltype = R_IA64_IMM64; break;
+	case EM_MIPS:	/* reltype: e_class    */ break;
+	case EM_PPC:	reltype = R_PPC_ADDR32; break;
+	case EM_PPC64:	reltype = R_PPC64_ADDR64; break;
+	case EM_S390:	/* reltype: e_class    */ break;
+	case EM_SH:	reltype = R_SH_DIR32; gpfx = 0; break;
+	case EM_SPARCV9: reltype = R_SPARC_64; break;
 	case EM_X86_64:
 		make_nop = make_nop_x86;
 		ideal_nop = ideal_nop5_x86_64;
 		reltype = R_X86_64_64;
 		rel_type_nop = R_X86_64_NONE;
 		mcount_adjust_64 = -1;
+		gpfx = 0;
 		break;
 	}  /* end switch */
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v4 7/8] recordmcount: Remove redundant cleanup() calls
  2019-07-31 18:24 [PATCH v4 0/8] recordmcount cleanups Matt Helsley
                   ` (5 preceding siblings ...)
  2019-07-31 18:24 ` [PATCH v4 6/8] recordmcount: Kernel style formatting Matt Helsley
@ 2019-07-31 18:24 ` Matt Helsley
  2019-07-31 18:24 ` [PATCH v4 8/8] recordmcount: Clarify what cleanup() does Matt Helsley
  2019-08-02 17:47 ` [PATCH v4 0/8] recordmcount cleanups Steven Rostedt
  8 siblings, 0 replies; 20+ messages in thread
From: Matt Helsley @ 2019-07-31 18:24 UTC (permalink / raw)
  To: LKML; +Cc: Ingo Molnar, Steven Rostedt, Matt Helsley

Redundant cleanup calls were introduced when transitioning from
the old error/success handling via setjmp/longjmp -- the longjmp
ensured the cleanup() call only happened once but replacing
the success_file()/fail_file() calls with cleanup() meant that
multiple cleanup() calls can happen as we return from function
calls.

In do_file(), looking just before and after the "goto out" jumps we
can see that multiple cleanups() are being performed. We remove
cleanup() calls from the nested functions because it makes the code
easier to review -- the resources being cleaned up are generally
allocated and initialized in the callers so freeing them there
makes more sense.

Other redundant cleanup() calls:

mmap_file() is only called from do_file() and, if mmap_file() fails,
then we goto out and do cleanup() there too.

write_file() is only called from do_file() and do_file()
calls cleanup() unconditionally after returning from write_file()
therefore the cleanup() calls in write_file() are not necessary.

find_secsym_ndx(), called from do_func()'s for-loop, when we are
cleaning up here it's obvious that we break out of the loop and
do another cleanup().

__has_rel_mcount() is called from two parts of do_func()
and calls cleanup(). In theory we move them into do_func(), however
these in turn prove redundant so another simplification step
removes them as well.

Signed-off-by: Matt Helsley <mhelsley@vmware.com>
---
 scripts/recordmcount.c | 13 -------------
 scripts/recordmcount.h |  2 --
 2 files changed, 15 deletions(-)

diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index 273ca8b42b20..5677fcc88a72 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -258,17 +258,14 @@ static void *mmap_file(char const *fname)
 	fd_map = open(fname, O_RDONLY);
 	if (fd_map < 0) {
 		perror(fname);
-		cleanup();
 		return NULL;
 	}
 	if (fstat(fd_map, &sb) < 0) {
 		perror(fname);
-		cleanup();
 		goto out;
 	}
 	if (!S_ISREG(sb.st_mode)) {
 		fprintf(stderr, "not a regular file: %s\n", fname);
-		cleanup();
 		goto out;
 	}
 	file_map = mmap(0, sb.st_size, PROT_READ|PROT_WRITE, MAP_PRIVATE,
@@ -314,13 +311,11 @@ static int write_file(const char *fname)
 	fd_map = open(tmp_file, O_WRONLY | O_TRUNC | O_CREAT, sb.st_mode);
 	if (fd_map < 0) {
 		perror(fname);
-		cleanup();
 		return -1;
 	}
 	n = write(fd_map, file_map, sb.st_size);
 	if (n != sb.st_size) {
 		perror("write");
-		cleanup();
 		close(fd_map);
 		return -1;
 	}
@@ -328,7 +323,6 @@ static int write_file(const char *fname)
 		n = write(fd_map, file_append, file_append_size);
 		if (n != file_append_size) {
 			perror("write");
-			cleanup();
 			close(fd_map);
 			return -1;
 		}
@@ -336,7 +330,6 @@ static int write_file(const char *fname)
 	close(fd_map);
 	if (rename(tmp_file, fname) < 0) {
 		perror(fname);
-		cleanup();
 		return -1;
 	}
 	return 0;
@@ -460,7 +453,6 @@ static int do_file(char const *const fname)
 	default:
 		fprintf(stderr, "unrecognized ELF data encoding %d: %s\n",
 			ehdr->e_ident[EI_DATA], fname);
-		cleanup();
 		goto out;
 	case ELFDATA2LSB:
 		if (*(unsigned char const *)&endian != 1) {
@@ -493,7 +485,6 @@ static int do_file(char const *const fname)
 	    w2(ehdr->e_type) != ET_REL ||
 	    ehdr->e_ident[EI_VERSION] != EV_CURRENT) {
 		fprintf(stderr, "unrecognized ET_REL file %s\n", fname);
-		cleanup();
 		goto out;
 	}
 
@@ -502,7 +493,6 @@ static int do_file(char const *const fname)
 	default:
 		fprintf(stderr, "unrecognized e_machine %u %s\n",
 			w2(ehdr->e_machine), fname);
-		cleanup();
 		goto out;
 	case EM_386:
 		reltype = R_386_32;
@@ -546,14 +536,12 @@ static int do_file(char const *const fname)
 	default:
 		fprintf(stderr, "unrecognized ELF class %d %s\n",
 			ehdr->e_ident[EI_CLASS], fname);
-		cleanup();
 		goto out;
 	case ELFCLASS32:
 		if (w2(ehdr->e_ehsize) != sizeof(Elf32_Ehdr)
 		||  w2(ehdr->e_shentsize) != sizeof(Elf32_Shdr)) {
 			fprintf(stderr,
 				"unrecognized ET_REL file: %s\n", fname);
-			cleanup();
 			goto out;
 		}
 		if (w2(ehdr->e_machine) == EM_MIPS) {
@@ -569,7 +557,6 @@ static int do_file(char const *const fname)
 		||  w2(ghdr->e_shentsize) != sizeof(Elf64_Shdr)) {
 			fprintf(stderr,
 				"unrecognized ET_REL file: %s\n", fname);
-			cleanup();
 			goto out;
 		}
 		if (w2(ghdr->e_machine) == EM_S390) {
diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index ca9aaac89bfb..8f0a278ce0af 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -463,7 +463,6 @@ static int find_secsym_ndx(unsigned const txtndx,
 	}
 	fprintf(stderr, "Cannot find symbol for section %u: %s.\n",
 		txtndx, txtname);
-	cleanup();
 	return -1;
 }
 
@@ -480,7 +479,6 @@ static char const * __has_rel_mcount(Elf_Shdr const *const relhdr, /* reltype */
 	if (strcmp("__mcount_loc", txtname) == 0) {
 		fprintf(stderr, "warning: __mcount_loc already exists: %s\n",
 			fname);
-		cleanup();
 		return already_has_rel_mcount;
 	}
 	if (w(txthdr->sh_type) != SHT_PROGBITS ||
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH v4 8/8] recordmcount: Clarify what cleanup() does
  2019-07-31 18:24 [PATCH v4 0/8] recordmcount cleanups Matt Helsley
                   ` (6 preceding siblings ...)
  2019-07-31 18:24 ` [PATCH v4 7/8] recordmcount: Remove redundant cleanup() calls Matt Helsley
@ 2019-07-31 18:24 ` Matt Helsley
  2019-08-02 17:47 ` [PATCH v4 0/8] recordmcount cleanups Steven Rostedt
  8 siblings, 0 replies; 20+ messages in thread
From: Matt Helsley @ 2019-07-31 18:24 UTC (permalink / raw)
  To: LKML; +Cc: Ingo Molnar, Steven Rostedt, Matt Helsley

cleanup() mostly frees/unmaps the malloc'd/privately-mapped
copy of the ELF file recordmcount is working on, which is
set up in mmap_file(). It also deals with positioning within
the pseduo prive-mapping of the file and appending to the ELF
file.

Split into two steps:
	mmap_cleanup() for the mapping itself
	file_append_cleanup() for allocations storing the
		appended ELF data.

Also, move the global variable initializations out of the main,
per-object-file loop and nearer to the alloc/init (mmap_file())
and two cleanup functions so we can more clearly see how they're
related.

Signed-off-by: Matt Helsley <mhelsley@vmware.com>
---
 scripts/recordmcount.c | 151 ++++++++++++++++++++++-------------------
 1 file changed, 81 insertions(+), 70 deletions(-)

diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index 5677fcc88a72..612268eabef4 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -48,21 +48,26 @@ static void *file_map;	/* pointer of the mapped file */
 static void *file_end;	/* pointer to the end of the mapped file */
 static int file_updated; /* flag to state file was changed */
 static void *file_ptr;	/* current file pointer location */
+
 static void *file_append; /* added to the end of the file */
 static size_t file_append_size; /* how much is added to end of file */
 
 /* Per-file resource cleanup when multiple files. */
-static void cleanup(void)
+static void file_append_cleanup(void)
+{
+	free(file_append);
+	file_append = NULL;
+	file_append_size = 0;
+	file_updated = 0;
+}
+
+static void mmap_cleanup(void)
 {
 	if (!mmap_failed)
 		munmap(file_map, sb.st_size);
 	else
 		free(file_map);
 	file_map = NULL;
-	free(file_append);
-	file_append = NULL;
-	file_append_size = 0;
-	file_updated = 0;
 }
 
 /* ulseek, uwrite, ...:  Check return value for errors. */
@@ -103,7 +108,8 @@ static ssize_t uwrite(void const *const buf, size_t const count)
 		}
 		if (!file_append) {
 			perror("write");
-			cleanup();
+			file_append_cleanup();
+			mmap_cleanup();
 			return -1;
 		}
 		if (file_ptr < file_end) {
@@ -129,12 +135,76 @@ static void * umalloc(size_t size)
 	void *const addr = malloc(size);
 	if (addr == 0) {
 		fprintf(stderr, "malloc failed: %zu bytes\n", size);
-		cleanup();
+		file_append_cleanup();
+		mmap_cleanup();
 		return NULL;
 	}
 	return addr;
 }
 
+/*
+ * Get the whole file as a programming convenience in order to avoid
+ * malloc+lseek+read+free of many pieces.  If successful, then mmap
+ * avoids copying unused pieces; else just read the whole file.
+ * Open for both read and write; new info will be appended to the file.
+ * Use MAP_PRIVATE so that a few changes to the in-memory ElfXX_Ehdr
+ * do not propagate to the file until an explicit overwrite at the last.
+ * This preserves most aspects of consistency (all except .st_size)
+ * for simultaneous readers of the file while we are appending to it.
+ * However, multiple writers still are bad.  We choose not to use
+ * locking because it is expensive and the use case of kernel build
+ * makes multiple writers unlikely.
+ */
+static void *mmap_file(char const *fname)
+{
+	/* Avoid problems if early cleanup() */
+	fd_map = -1;
+	mmap_failed = 1;
+	file_map = NULL;
+	file_ptr = NULL;
+	file_updated = 0;
+	sb.st_size = 0;
+
+	fd_map = open(fname, O_RDONLY);
+	if (fd_map < 0) {
+		perror(fname);
+		return NULL;
+	}
+	if (fstat(fd_map, &sb) < 0) {
+		perror(fname);
+		goto out;
+	}
+	if (!S_ISREG(sb.st_mode)) {
+		fprintf(stderr, "not a regular file: %s\n", fname);
+		goto out;
+	}
+	file_map = mmap(0, sb.st_size, PROT_READ|PROT_WRITE, MAP_PRIVATE,
+			fd_map, 0);
+	if (file_map == MAP_FAILED) {
+		mmap_failed = 1;
+		file_map = umalloc(sb.st_size);
+		if (!file_map) {
+			perror(fname);
+			goto out;
+		}
+		if (read(fd_map, file_map, sb.st_size) != sb.st_size) {
+			perror(fname);
+			free(file_map);
+			file_map = NULL;
+			goto out;
+		}
+	} else
+		mmap_failed = 0;
+out:
+	close(fd_map);
+	fd_map = -1;
+
+	file_end = file_map + sb.st_size;
+
+	return file_map;
+}
+
+
 static unsigned char ideal_nop5_x86_64[5] = { 0x0f, 0x1f, 0x44, 0x00, 0x00 };
 static unsigned char ideal_nop5_x86_32[5] = { 0x3e, 0x8d, 0x74, 0x26, 0x00 };
 static unsigned char *ideal_nop;
@@ -238,61 +308,6 @@ static int make_nop_arm64(void *map, size_t const offset)
 	return 0;
 }
 
-/*
- * Get the whole file as a programming convenience in order to avoid
- * malloc+lseek+read+free of many pieces.  If successful, then mmap
- * avoids copying unused pieces; else just read the whole file.
- * Open for both read and write; new info will be appended to the file.
- * Use MAP_PRIVATE so that a few changes to the in-memory ElfXX_Ehdr
- * do not propagate to the file until an explicit overwrite at the last.
- * This preserves most aspects of consistency (all except .st_size)
- * for simultaneous readers of the file while we are appending to it.
- * However, multiple writers still are bad.  We choose not to use
- * locking because it is expensive and the use case of kernel build
- * makes multiple writers unlikely.
- */
-static void *mmap_file(char const *fname)
-{
-	file_map = NULL;
-	sb.st_size = 0;
-	fd_map = open(fname, O_RDONLY);
-	if (fd_map < 0) {
-		perror(fname);
-		return NULL;
-	}
-	if (fstat(fd_map, &sb) < 0) {
-		perror(fname);
-		goto out;
-	}
-	if (!S_ISREG(sb.st_mode)) {
-		fprintf(stderr, "not a regular file: %s\n", fname);
-		goto out;
-	}
-	file_map = mmap(0, sb.st_size, PROT_READ|PROT_WRITE, MAP_PRIVATE,
-			fd_map, 0);
-	mmap_failed = 0;
-	if (file_map == MAP_FAILED) {
-		mmap_failed = 1;
-		file_map = umalloc(sb.st_size);
-		if (!file_map) {
-			perror(fname);
-			goto out;
-		}
-		if (read(fd_map, file_map, sb.st_size) != sb.st_size) {
-			perror(fname);
-			free(file_map);
-			file_map = NULL;
-			goto out;
-		}
-	}
-out:
-	close(fd_map);
-
-	file_end = file_map + sb.st_size;
-
-	return file_map;
-}
-
 static int write_file(const char *fname)
 {
 	char tmp_file[strlen(fname) + 4];
@@ -438,10 +453,11 @@ static void MIPS64_r_info(Elf64_Rel *const rp, unsigned sym, unsigned type)
 
 static int do_file(char const *const fname)
 {
-	Elf32_Ehdr *const ehdr = mmap_file(fname);
 	unsigned int reltype = 0;
+	Elf32_Ehdr *ehdr;
 	int rc = -1;
 
+	ehdr = mmap_file(fname);
 	if (!ehdr)
 		goto out;
 
@@ -577,7 +593,8 @@ static int do_file(char const *const fname)
 
 	rc = write_file(fname);
 out:
-	cleanup();
+	file_append_cleanup();
+	mmap_cleanup();
 	return rc;
 }
 
@@ -620,12 +637,6 @@ int main(int argc, char *argv[])
 		    strcmp(file + (len - ftrace_size), ftrace) == 0)
 			continue;
 
-		/* Avoid problems if early cleanup() */
-		fd_map = -1;
-		mmap_failed = 1;
-		file_map = NULL;
-		file_ptr = NULL;
-		file_updated = 0;
 		if (do_file(file)) {
 			fprintf(stderr, "%s: failed\n", file);
 			++n_error;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* Re: [PATCH v4 0/8] recordmcount cleanups
  2019-07-31 18:24 [PATCH v4 0/8] recordmcount cleanups Matt Helsley
                   ` (7 preceding siblings ...)
  2019-07-31 18:24 ` [PATCH v4 8/8] recordmcount: Clarify what cleanup() does Matt Helsley
@ 2019-08-02 17:47 ` Steven Rostedt
  8 siblings, 0 replies; 20+ messages in thread
From: Steven Rostedt @ 2019-08-02 17:47 UTC (permalink / raw)
  To: Matt Helsley; +Cc: LKML, Ingo Molnar

On Wed, 31 Jul 2019 11:24:08 -0700
Matt Helsley <mhelsley@vmware.com> wrote:

> recordmcount presents unnecessary challenges to reviewers:
> 
> 	It pretends to wrap access to the ELF file in
> 	uread/uwrite/ulseek functions which aren't related
> 	the way you might think (i.e. not the way read, write,
> 	and lseek are releated to each other).
> 
> 	It uses setjmp/longjmp to handle errors (and success)
> 	during processing of the object files. This makes it
> 	hard to review what functions are doing, how globals
> 	change over time, etc.
> 
> 	There are some kernel style nits.
> 
> This series addresses all of those by removing un-helper functions,
> unused parameters, and rewriting the error/success handling to
> better resemble regular kernel C code.
> 

I applied these patches to my queue.

I pushed them to my repo on branch ftrace/core

  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git

But note, that this branch will rebase, probably on top of v5.3-rc3
when it comes out.

-- Steve

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v4 4/8] recordmcount: Rewrite error/success handling
  2019-07-31 18:24 ` [PATCH v4 4/8] recordmcount: Rewrite error/success handling Matt Helsley
@ 2019-10-09 10:46     ` Uwe Kleine-König
  0 siblings, 0 replies; 20+ messages in thread
From: Uwe Kleine-König @ 2019-10-09 10:46 UTC (permalink / raw)
  To: Matt Helsley; +Cc: LKML, Ingo Molnar, Steven Rostedt, kernel, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 2272 bytes --]

Hello,

On Wed, Jul 31, 2019 at 11:24:12AM -0700, Matt Helsley wrote:
> Recordmcount uses setjmp/longjmp to manage control flow as
> it reads and then writes the ELF file. This unusual control
> flow is hard to follow and check in addition to being unlike
> kernel coding style.
> 
> So we rewrite these paths to use regular return values to
> indicate error/success. When an error or previously-completed object
> file is found we return an error code following kernel
> coding conventions -- negative error values and 0 for success when
> we're not returning a pointer. We return NULL for those that fail
> and return non-NULL pointers otherwise.
> 
> One oddity is already_has_rel_mcount -- there we use pointer comparison
> rather than string comparison to differentiate between
> previously-processed object files and returning the name of a text
> section.
> 
> Signed-off-by: Matt Helsley <mhelsley@vmware.com>

This is 3f1df12019f333442b12c3b5d110b8fc43eb0b36 in mainline now.

I have the following problem:

	$ make ARCH=arm arch/arm/crypto/
	  Using /home/uwe/gsrc/linux as source for kernel
	  GEN     Makefile
	  CALL    /home/uwe/gsrc/linux/scripts/checksyscalls.sh
	  CALL    /home/uwe/gsrc/linux/scripts/atomic/check-atomics.sh
	  CC      arch/arm/crypto/aes-cipher-glue.o
	arch/arm/crypto/aes-cipher-glue.o: failed
	make[2]: *** [/home/uwe/gsrc/linux/scripts/Makefile.build:281: arch/arm/crypto/aes-cipher-glue.o] Error 1
	make[2]: *** Deleting file 'arch/arm/crypto/aes-cipher-glue.o'
	make[1]: *** [/home/uwe/gsrc/linux/Makefile:1792: arch/arm/crypto/] Error 2
	make: *** [/home/uwe/gsrc/linux/Makefile:179: sub-make] Error 2

and bisection points to 3f1df12019f333442b12c3b5d110b8fc43eb0b36.

This doesn't affect all files, most compile just fine. After calling the
compiler by hand to get aes-cipher-glue.o back I have:

uwe@taurus:~/arietta/kbuild$ ./scripts/recordmcount "arch/arm/crypto/aes-cipher-glue.o"
arch/arm/crypto/aes-cipher-glue.o: failed

I didn't debug this further, if you have problems reproducing or need more
infos tell me. The defconfig I'm using is attached.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: arietta_defconfig --]
[-- Type: text/plain, Size: 4859 bytes --]

# CONFIG_LOCALVERSION_AUTO is not set
# CONFIG_SWAP is not set
CONFIG_SYSVIPC=y
CONFIG_HIGH_RES_TIMERS=y
CONFIG_IKCONFIG=y
CONFIG_IKCONFIG_PROC=y
CONFIG_CGROUPS=y
CONFIG_CGROUP_BPF=y
CONFIG_NAMESPACES=y
CONFIG_BLK_DEV_INITRD=y
CONFIG_CC_OPTIMIZE_FOR_SIZE=y
CONFIG_KALLSYMS_ALL=y
CONFIG_BPF_SYSCALL=y
CONFIG_EMBEDDED=y
CONFIG_SLAB=y
CONFIG_ARCH_MULTI_V4T=y
CONFIG_ARCH_MULTI_V5=y
# CONFIG_ARCH_MULTI_V7 is not set
CONFIG_ARCH_AT91=y
CONFIG_SOC_AT91RM9200=y
CONFIG_SOC_AT91SAM9=y
CONFIG_AEABI=y
CONFIG_UACCESS_WITH_MEMCPY=y
CONFIG_SECCOMP=y
CONFIG_ZBOOT_ROM_TEXT=0x0
CONFIG_ZBOOT_ROM_BSS=0x0
CONFIG_ARM_APPENDED_DTB=y
CONFIG_ARM_ATAG_DTB_COMPAT=y
CONFIG_KEXEC=y
CONFIG_CPU_IDLE=y
CONFIG_ARM_CRYPTO=y
CONFIG_CRYPTO_SHA1_ARM=y
CONFIG_CRYPTO_SHA256_ARM=y
CONFIG_CRYPTO_SHA512_ARM=y
CONFIG_CRYPTO_AES_ARM=y
CONFIG_MODULES=y
CONFIG_MODULE_UNLOAD=y
# CONFIG_BLK_DEV_BSG is not set
# CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set
CONFIG_NET=y
CONFIG_PACKET=y
CONFIG_UNIX=y
CONFIG_INET=y
CONFIG_IP_MULTICAST=y
CONFIG_IP_PNP=y
CONFIG_IP_PNP_DHCP=y
CONFIG_IP_PNP_BOOTP=y
CONFIG_IP_PNP_RARP=y
# CONFIG_INET_DIAG is not set
CONFIG_IPV6_SIT_6RD=y
CONFIG_CFG80211=y
CONFIG_MAC80211=y
CONFIG_DEVTMPFS=y
CONFIG_DEVTMPFS_MOUNT=y
# CONFIG_STANDALONE is not set
# CONFIG_PREVENT_FIRMWARE_BUILD is not set
CONFIG_MTD=y
CONFIG_MTD_CMDLINE_PARTS=y
CONFIG_MTD_BLOCK=y
CONFIG_MTD_DATAFLASH=y
CONFIG_MTD_UBI=y
CONFIG_MTD_UBI_GLUEBI=y
CONFIG_BLK_DEV_LOOP=y
CONFIG_BLK_DEV_RAM=y
CONFIG_BLK_DEV_RAM_COUNT=4
CONFIG_BLK_DEV_RAM_SIZE=8192
CONFIG_ATMEL_TCLIB=y
CONFIG_ATMEL_SSC=y
CONFIG_EEPROM_93CX6=y
CONFIG_SCSI=y
CONFIG_BLK_DEV_SD=y
# CONFIG_SCSI_LOWLEVEL is not set
CONFIG_NETDEVICES=y
# CONFIG_NET_VENDOR_BROADCOM is not set
CONFIG_MACB=y
CONFIG_DM9000=y
# CONFIG_NET_VENDOR_FARADAY is not set
# CONFIG_NET_VENDOR_INTEL is not set
# CONFIG_NET_VENDOR_MARVELL is not set
# CONFIG_NET_VENDOR_MICREL is not set
# CONFIG_NET_VENDOR_NATSEMI is not set
# CONFIG_NET_VENDOR_SEEQ is not set
# CONFIG_NET_VENDOR_SMSC is not set
# CONFIG_NET_VENDOR_STMICRO is not set
CONFIG_DAVICOM_PHY=y
CONFIG_MICREL_PHY=y
CONFIG_RT2X00=y
CONFIG_RT2500USB=y
CONFIG_RT73USB=y
CONFIG_RT2800USB=y
CONFIG_RT2800USB_RT53XX=y
CONFIG_RT2800USB_RT55XX=y
CONFIG_RT2800USB_UNKNOWN=y
# CONFIG_RTL_CARDS is not set
CONFIG_INPUT_POLLDEV=y
CONFIG_INPUT_JOYDEV=y
CONFIG_INPUT_EVDEV=y
# CONFIG_KEYBOARD_ATKBD is not set
CONFIG_KEYBOARD_QT1070=y
CONFIG_KEYBOARD_GPIO=y
# CONFIG_INPUT_MOUSE is not set
CONFIG_INPUT_TOUCHSCREEN=y
CONFIG_TOUCHSCREEN_ADS7846=y
# CONFIG_SERIO is not set
CONFIG_LEGACY_PTY_COUNT=4
CONFIG_SERIAL_ATMEL=y
CONFIG_SERIAL_ATMEL_CONSOLE=y
CONFIG_HW_RANDOM=y
CONFIG_I2C=y
CONFIG_I2C_CHARDEV=y
CONFIG_I2C_AT91=y
CONFIG_I2C_GPIO=y
CONFIG_SPI=y
CONFIG_SPI_ATMEL=y
CONFIG_SPI_GPIO=y
CONFIG_SPI_SPIDEV=y
CONFIG_GPIO_SYSFS=y
CONFIG_W1=y
CONFIG_W1_MASTER_GPIO=y
CONFIG_W1_SLAVE_THERM=y
CONFIG_POWER_RESET=y
CONFIG_POWER_SUPPLY=y
CONFIG_SENSORS_SHT3x=y
CONFIG_WATCHDOG=y
CONFIG_AT91SAM9X_WATCHDOG=y
CONFIG_REGULATOR=y
CONFIG_REGULATOR_FIXED_VOLTAGE=y
CONFIG_FB=y
CONFIG_FB_ATMEL=y
# CONFIG_LCD_CLASS_DEVICE is not set
# CONFIG_BACKLIGHT_GENERIC is not set
CONFIG_FRAMEBUFFER_CONSOLE=y
CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY=y
CONFIG_SOUND=y
CONFIG_SND=y
CONFIG_SND_SOC=y
CONFIG_SND_ATMEL_SOC=y
CONFIG_SND_AT91_SOC_SAM9G20_WM8731=y
CONFIG_SND_ATMEL_SOC_WM8904=y
CONFIG_SND_AT91_SOC_SAM9X5_WM8731=y
CONFIG_USB=y
CONFIG_USB_ANNOUNCE_NEW_DEVICES=y
CONFIG_USB_EHCI_HCD=y
CONFIG_USB_OHCI_HCD=y
CONFIG_USB_ACM=y
CONFIG_USB_STORAGE=y
CONFIG_USB_SERIAL=y
CONFIG_USB_SERIAL_GENERIC=y
CONFIG_USB_SERIAL_FTDI_SIO=y
CONFIG_USB_SERIAL_PL2303=y
CONFIG_USB_GADGET=y
CONFIG_USB_AT91=y
CONFIG_USB_ATMEL_USBA=y
CONFIG_USB_ETH=y
CONFIG_MMC=y
CONFIG_MMC_ATMELMCI=y
CONFIG_MMC_SPI=y
CONFIG_NEW_LEDS=y
CONFIG_LEDS_CLASS=y
CONFIG_LEDS_GPIO=y
CONFIG_LEDS_PWM=y
CONFIG_LEDS_TRIGGERS=y
CONFIG_LEDS_TRIGGER_TIMER=y
CONFIG_LEDS_TRIGGER_HEARTBEAT=y
CONFIG_LEDS_TRIGGER_GPIO=y
CONFIG_RTC_CLASS=y
CONFIG_RTC_DRV_S35390A=y
CONFIG_RTC_DRV_RV3029C2=y
CONFIG_RTC_DRV_AT91RM9200=y
CONFIG_RTC_DRV_AT91SAM9=y
CONFIG_DMADEVICES=y
CONFIG_AT_HDMAC=y
# CONFIG_IOMMU_SUPPORT is not set
CONFIG_IIO=y
CONFIG_AT91_ADC=y
CONFIG_PWM=y
CONFIG_PWM_DEBUG=y
CONFIG_PWM_ATMEL=y
CONFIG_PWM_ATMEL_TCB=y
CONFIG_EXT4_FS=y
CONFIG_FANOTIFY=y
CONFIG_AUTOFS4_FS=y
CONFIG_VFAT_FS=y
CONFIG_TMPFS=y
CONFIG_UBIFS_FS=y
CONFIG_UBIFS_FS_ADVANCED_COMPR=y
CONFIG_NFS_FS=y
CONFIG_ROOT_NFS=y
CONFIG_NLS_CODEPAGE_437=y
CONFIG_NLS_CODEPAGE_850=y
CONFIG_NLS_ISO8859_1=y
CONFIG_NLS_UTF8=y
CONFIG_CRYPTO_ECHAINIV=y
CONFIG_CRYPTO_ECB=y
CONFIG_CRYPTO_USER_API_HASH=y
CONFIG_CRYPTO_USER_API_SKCIPHER=y
# CONFIG_CRYPTO_HW is not set
CONFIG_FONTS=y
CONFIG_FONT_8x8=y
CONFIG_FONT_ACORN_8x8=y
CONFIG_FONT_MINI_4x6=y
CONFIG_PRINTK_TIME=y
CONFIG_STRIP_ASM_SYMS=y
# CONFIG_SCHED_DEBUG is not set
# CONFIG_DEBUG_BUGVERBOSE is not set
CONFIG_FUNCTION_TRACER=y
CONFIG_TEST_PRINTF=y
CONFIG_DEBUG_USER=y

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v4 4/8] recordmcount: Rewrite error/success handling
@ 2019-10-09 10:46     ` Uwe Kleine-König
  0 siblings, 0 replies; 20+ messages in thread
From: Uwe Kleine-König @ 2019-10-09 10:46 UTC (permalink / raw)
  To: Matt Helsley; +Cc: kernel, linux-arm-kernel, LKML, Steven Rostedt, Ingo Molnar

[-- Attachment #1: Type: text/plain, Size: 2272 bytes --]

Hello,

On Wed, Jul 31, 2019 at 11:24:12AM -0700, Matt Helsley wrote:
> Recordmcount uses setjmp/longjmp to manage control flow as
> it reads and then writes the ELF file. This unusual control
> flow is hard to follow and check in addition to being unlike
> kernel coding style.
> 
> So we rewrite these paths to use regular return values to
> indicate error/success. When an error or previously-completed object
> file is found we return an error code following kernel
> coding conventions -- negative error values and 0 for success when
> we're not returning a pointer. We return NULL for those that fail
> and return non-NULL pointers otherwise.
> 
> One oddity is already_has_rel_mcount -- there we use pointer comparison
> rather than string comparison to differentiate between
> previously-processed object files and returning the name of a text
> section.
> 
> Signed-off-by: Matt Helsley <mhelsley@vmware.com>

This is 3f1df12019f333442b12c3b5d110b8fc43eb0b36 in mainline now.

I have the following problem:

	$ make ARCH=arm arch/arm/crypto/
	  Using /home/uwe/gsrc/linux as source for kernel
	  GEN     Makefile
	  CALL    /home/uwe/gsrc/linux/scripts/checksyscalls.sh
	  CALL    /home/uwe/gsrc/linux/scripts/atomic/check-atomics.sh
	  CC      arch/arm/crypto/aes-cipher-glue.o
	arch/arm/crypto/aes-cipher-glue.o: failed
	make[2]: *** [/home/uwe/gsrc/linux/scripts/Makefile.build:281: arch/arm/crypto/aes-cipher-glue.o] Error 1
	make[2]: *** Deleting file 'arch/arm/crypto/aes-cipher-glue.o'
	make[1]: *** [/home/uwe/gsrc/linux/Makefile:1792: arch/arm/crypto/] Error 2
	make: *** [/home/uwe/gsrc/linux/Makefile:179: sub-make] Error 2

and bisection points to 3f1df12019f333442b12c3b5d110b8fc43eb0b36.

This doesn't affect all files, most compile just fine. After calling the
compiler by hand to get aes-cipher-glue.o back I have:

uwe@taurus:~/arietta/kbuild$ ./scripts/recordmcount "arch/arm/crypto/aes-cipher-glue.o"
arch/arm/crypto/aes-cipher-glue.o: failed

I didn't debug this further, if you have problems reproducing or need more
infos tell me. The defconfig I'm using is attached.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: arietta_defconfig --]
[-- Type: text/plain, Size: 4859 bytes --]

# CONFIG_LOCALVERSION_AUTO is not set
# CONFIG_SWAP is not set
CONFIG_SYSVIPC=y
CONFIG_HIGH_RES_TIMERS=y
CONFIG_IKCONFIG=y
CONFIG_IKCONFIG_PROC=y
CONFIG_CGROUPS=y
CONFIG_CGROUP_BPF=y
CONFIG_NAMESPACES=y
CONFIG_BLK_DEV_INITRD=y
CONFIG_CC_OPTIMIZE_FOR_SIZE=y
CONFIG_KALLSYMS_ALL=y
CONFIG_BPF_SYSCALL=y
CONFIG_EMBEDDED=y
CONFIG_SLAB=y
CONFIG_ARCH_MULTI_V4T=y
CONFIG_ARCH_MULTI_V5=y
# CONFIG_ARCH_MULTI_V7 is not set
CONFIG_ARCH_AT91=y
CONFIG_SOC_AT91RM9200=y
CONFIG_SOC_AT91SAM9=y
CONFIG_AEABI=y
CONFIG_UACCESS_WITH_MEMCPY=y
CONFIG_SECCOMP=y
CONFIG_ZBOOT_ROM_TEXT=0x0
CONFIG_ZBOOT_ROM_BSS=0x0
CONFIG_ARM_APPENDED_DTB=y
CONFIG_ARM_ATAG_DTB_COMPAT=y
CONFIG_KEXEC=y
CONFIG_CPU_IDLE=y
CONFIG_ARM_CRYPTO=y
CONFIG_CRYPTO_SHA1_ARM=y
CONFIG_CRYPTO_SHA256_ARM=y
CONFIG_CRYPTO_SHA512_ARM=y
CONFIG_CRYPTO_AES_ARM=y
CONFIG_MODULES=y
CONFIG_MODULE_UNLOAD=y
# CONFIG_BLK_DEV_BSG is not set
# CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set
CONFIG_NET=y
CONFIG_PACKET=y
CONFIG_UNIX=y
CONFIG_INET=y
CONFIG_IP_MULTICAST=y
CONFIG_IP_PNP=y
CONFIG_IP_PNP_DHCP=y
CONFIG_IP_PNP_BOOTP=y
CONFIG_IP_PNP_RARP=y
# CONFIG_INET_DIAG is not set
CONFIG_IPV6_SIT_6RD=y
CONFIG_CFG80211=y
CONFIG_MAC80211=y
CONFIG_DEVTMPFS=y
CONFIG_DEVTMPFS_MOUNT=y
# CONFIG_STANDALONE is not set
# CONFIG_PREVENT_FIRMWARE_BUILD is not set
CONFIG_MTD=y
CONFIG_MTD_CMDLINE_PARTS=y
CONFIG_MTD_BLOCK=y
CONFIG_MTD_DATAFLASH=y
CONFIG_MTD_UBI=y
CONFIG_MTD_UBI_GLUEBI=y
CONFIG_BLK_DEV_LOOP=y
CONFIG_BLK_DEV_RAM=y
CONFIG_BLK_DEV_RAM_COUNT=4
CONFIG_BLK_DEV_RAM_SIZE=8192
CONFIG_ATMEL_TCLIB=y
CONFIG_ATMEL_SSC=y
CONFIG_EEPROM_93CX6=y
CONFIG_SCSI=y
CONFIG_BLK_DEV_SD=y
# CONFIG_SCSI_LOWLEVEL is not set
CONFIG_NETDEVICES=y
# CONFIG_NET_VENDOR_BROADCOM is not set
CONFIG_MACB=y
CONFIG_DM9000=y
# CONFIG_NET_VENDOR_FARADAY is not set
# CONFIG_NET_VENDOR_INTEL is not set
# CONFIG_NET_VENDOR_MARVELL is not set
# CONFIG_NET_VENDOR_MICREL is not set
# CONFIG_NET_VENDOR_NATSEMI is not set
# CONFIG_NET_VENDOR_SEEQ is not set
# CONFIG_NET_VENDOR_SMSC is not set
# CONFIG_NET_VENDOR_STMICRO is not set
CONFIG_DAVICOM_PHY=y
CONFIG_MICREL_PHY=y
CONFIG_RT2X00=y
CONFIG_RT2500USB=y
CONFIG_RT73USB=y
CONFIG_RT2800USB=y
CONFIG_RT2800USB_RT53XX=y
CONFIG_RT2800USB_RT55XX=y
CONFIG_RT2800USB_UNKNOWN=y
# CONFIG_RTL_CARDS is not set
CONFIG_INPUT_POLLDEV=y
CONFIG_INPUT_JOYDEV=y
CONFIG_INPUT_EVDEV=y
# CONFIG_KEYBOARD_ATKBD is not set
CONFIG_KEYBOARD_QT1070=y
CONFIG_KEYBOARD_GPIO=y
# CONFIG_INPUT_MOUSE is not set
CONFIG_INPUT_TOUCHSCREEN=y
CONFIG_TOUCHSCREEN_ADS7846=y
# CONFIG_SERIO is not set
CONFIG_LEGACY_PTY_COUNT=4
CONFIG_SERIAL_ATMEL=y
CONFIG_SERIAL_ATMEL_CONSOLE=y
CONFIG_HW_RANDOM=y
CONFIG_I2C=y
CONFIG_I2C_CHARDEV=y
CONFIG_I2C_AT91=y
CONFIG_I2C_GPIO=y
CONFIG_SPI=y
CONFIG_SPI_ATMEL=y
CONFIG_SPI_GPIO=y
CONFIG_SPI_SPIDEV=y
CONFIG_GPIO_SYSFS=y
CONFIG_W1=y
CONFIG_W1_MASTER_GPIO=y
CONFIG_W1_SLAVE_THERM=y
CONFIG_POWER_RESET=y
CONFIG_POWER_SUPPLY=y
CONFIG_SENSORS_SHT3x=y
CONFIG_WATCHDOG=y
CONFIG_AT91SAM9X_WATCHDOG=y
CONFIG_REGULATOR=y
CONFIG_REGULATOR_FIXED_VOLTAGE=y
CONFIG_FB=y
CONFIG_FB_ATMEL=y
# CONFIG_LCD_CLASS_DEVICE is not set
# CONFIG_BACKLIGHT_GENERIC is not set
CONFIG_FRAMEBUFFER_CONSOLE=y
CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY=y
CONFIG_SOUND=y
CONFIG_SND=y
CONFIG_SND_SOC=y
CONFIG_SND_ATMEL_SOC=y
CONFIG_SND_AT91_SOC_SAM9G20_WM8731=y
CONFIG_SND_ATMEL_SOC_WM8904=y
CONFIG_SND_AT91_SOC_SAM9X5_WM8731=y
CONFIG_USB=y
CONFIG_USB_ANNOUNCE_NEW_DEVICES=y
CONFIG_USB_EHCI_HCD=y
CONFIG_USB_OHCI_HCD=y
CONFIG_USB_ACM=y
CONFIG_USB_STORAGE=y
CONFIG_USB_SERIAL=y
CONFIG_USB_SERIAL_GENERIC=y
CONFIG_USB_SERIAL_FTDI_SIO=y
CONFIG_USB_SERIAL_PL2303=y
CONFIG_USB_GADGET=y
CONFIG_USB_AT91=y
CONFIG_USB_ATMEL_USBA=y
CONFIG_USB_ETH=y
CONFIG_MMC=y
CONFIG_MMC_ATMELMCI=y
CONFIG_MMC_SPI=y
CONFIG_NEW_LEDS=y
CONFIG_LEDS_CLASS=y
CONFIG_LEDS_GPIO=y
CONFIG_LEDS_PWM=y
CONFIG_LEDS_TRIGGERS=y
CONFIG_LEDS_TRIGGER_TIMER=y
CONFIG_LEDS_TRIGGER_HEARTBEAT=y
CONFIG_LEDS_TRIGGER_GPIO=y
CONFIG_RTC_CLASS=y
CONFIG_RTC_DRV_S35390A=y
CONFIG_RTC_DRV_RV3029C2=y
CONFIG_RTC_DRV_AT91RM9200=y
CONFIG_RTC_DRV_AT91SAM9=y
CONFIG_DMADEVICES=y
CONFIG_AT_HDMAC=y
# CONFIG_IOMMU_SUPPORT is not set
CONFIG_IIO=y
CONFIG_AT91_ADC=y
CONFIG_PWM=y
CONFIG_PWM_DEBUG=y
CONFIG_PWM_ATMEL=y
CONFIG_PWM_ATMEL_TCB=y
CONFIG_EXT4_FS=y
CONFIG_FANOTIFY=y
CONFIG_AUTOFS4_FS=y
CONFIG_VFAT_FS=y
CONFIG_TMPFS=y
CONFIG_UBIFS_FS=y
CONFIG_UBIFS_FS_ADVANCED_COMPR=y
CONFIG_NFS_FS=y
CONFIG_ROOT_NFS=y
CONFIG_NLS_CODEPAGE_437=y
CONFIG_NLS_CODEPAGE_850=y
CONFIG_NLS_ISO8859_1=y
CONFIG_NLS_UTF8=y
CONFIG_CRYPTO_ECHAINIV=y
CONFIG_CRYPTO_ECB=y
CONFIG_CRYPTO_USER_API_HASH=y
CONFIG_CRYPTO_USER_API_SKCIPHER=y
# CONFIG_CRYPTO_HW is not set
CONFIG_FONTS=y
CONFIG_FONT_8x8=y
CONFIG_FONT_ACORN_8x8=y
CONFIG_FONT_MINI_4x6=y
CONFIG_PRINTK_TIME=y
CONFIG_STRIP_ASM_SYMS=y
# CONFIG_SCHED_DEBUG is not set
# CONFIG_DEBUG_BUGVERBOSE is not set
CONFIG_FUNCTION_TRACER=y
CONFIG_TEST_PRINTF=y
CONFIG_DEBUG_USER=y

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v4 4/8] recordmcount: Rewrite error/success handling
  2019-10-09 10:46     ` Uwe Kleine-König
@ 2019-10-09 15:05       ` Steven Rostedt
  -1 siblings, 0 replies; 20+ messages in thread
From: Steven Rostedt @ 2019-10-09 15:05 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Matt Helsley, LKML, Ingo Molnar, kernel, linux-arm-kernel

On Wed, 9 Oct 2019 12:46:26 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:


> uwe@taurus:~/arietta/kbuild$ ./scripts/recordmcount "arch/arm/crypto/aes-cipher-glue.o"
> arch/arm/crypto/aes-cipher-glue.o: failed

Thanks for the report.

> 
> I didn't debug this further, if you have problems reproducing or need more
> infos tell me. The defconfig I'm using is attached.
> 

Does this fix it for you?

-- Steve

diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index 3796eb37fb12..6dbec46b7703 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -389,11 +389,8 @@ static int nop_mcount(Elf_Shdr const *const relhdr,
 			mcountsym = get_mcountsym(sym0, relp, str0);
 
 		if (mcountsym == Elf_r_sym(relp) && !is_fake_mcount(relp)) {
-			if (make_nop) {
+			if (make_nop)
 				ret = make_nop((void *)ehdr, _w(shdr->sh_offset) + _w(relp->r_offset));
-				if (ret < 0)
-					return -1;
-			}
 			if (warn_on_notrace_sect && !once) {
 				printf("Section %s has mcount callers being ignored\n",
 				       txtname);

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* Re: [PATCH v4 4/8] recordmcount: Rewrite error/success handling
@ 2019-10-09 15:05       ` Steven Rostedt
  0 siblings, 0 replies; 20+ messages in thread
From: Steven Rostedt @ 2019-10-09 15:05 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Ingo Molnar, linux-arm-kernel, LKML, kernel, Matt Helsley

On Wed, 9 Oct 2019 12:46:26 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:


> uwe@taurus:~/arietta/kbuild$ ./scripts/recordmcount "arch/arm/crypto/aes-cipher-glue.o"
> arch/arm/crypto/aes-cipher-glue.o: failed

Thanks for the report.

> 
> I didn't debug this further, if you have problems reproducing or need more
> infos tell me. The defconfig I'm using is attached.
> 

Does this fix it for you?

-- Steve

diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index 3796eb37fb12..6dbec46b7703 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -389,11 +389,8 @@ static int nop_mcount(Elf_Shdr const *const relhdr,
 			mcountsym = get_mcountsym(sym0, relp, str0);
 
 		if (mcountsym == Elf_r_sym(relp) && !is_fake_mcount(relp)) {
-			if (make_nop) {
+			if (make_nop)
 				ret = make_nop((void *)ehdr, _w(shdr->sh_offset) + _w(relp->r_offset));
-				if (ret < 0)
-					return -1;
-			}
 			if (warn_on_notrace_sect && !once) {
 				printf("Section %s has mcount callers being ignored\n",
 				       txtname);

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* Re: [PATCH v4 4/8] recordmcount: Rewrite error/success handling
  2019-10-09 15:05       ` Steven Rostedt
@ 2019-10-09 15:22         ` Uwe Kleine-König
  -1 siblings, 0 replies; 20+ messages in thread
From: Uwe Kleine-König @ 2019-10-09 15:22 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Matt Helsley, LKML, Ingo Molnar, kernel, linux-arm-kernel

On Wed, Oct 09, 2019 at 11:05:38AM -0400, Steven Rostedt wrote:
> On Wed, 9 Oct 2019 12:46:26 +0200
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> 
> 
> > uwe@taurus:~/arietta/kbuild$ ./scripts/recordmcount "arch/arm/crypto/aes-cipher-glue.o"
> > arch/arm/crypto/aes-cipher-glue.o: failed
> 
> Thanks for the report.
> 
> > 
> > I didn't debug this further, if you have problems reproducing or need more
> > infos tell me. The defconfig I'm using is attached.
> > 
> 
> Does this fix it for you?
> 
> -- Steve
> 
> diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
> index 3796eb37fb12..6dbec46b7703 100644
> --- a/scripts/recordmcount.h
> +++ b/scripts/recordmcount.h
> @@ -389,11 +389,8 @@ static int nop_mcount(Elf_Shdr const *const relhdr,
>  			mcountsym = get_mcountsym(sym0, relp, str0);
>  
>  		if (mcountsym == Elf_r_sym(relp) && !is_fake_mcount(relp)) {
> -			if (make_nop) {
> +			if (make_nop)
>  				ret = make_nop((void *)ehdr, _w(shdr->sh_offset) + _w(relp->r_offset));
> -				if (ret < 0)
> -					return -1;
> -			}

Yes, this patch fixes building for me.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v4 4/8] recordmcount: Rewrite error/success handling
@ 2019-10-09 15:22         ` Uwe Kleine-König
  0 siblings, 0 replies; 20+ messages in thread
From: Uwe Kleine-König @ 2019-10-09 15:22 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Ingo Molnar, linux-arm-kernel, LKML, kernel, Matt Helsley

On Wed, Oct 09, 2019 at 11:05:38AM -0400, Steven Rostedt wrote:
> On Wed, 9 Oct 2019 12:46:26 +0200
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> 
> 
> > uwe@taurus:~/arietta/kbuild$ ./scripts/recordmcount "arch/arm/crypto/aes-cipher-glue.o"
> > arch/arm/crypto/aes-cipher-glue.o: failed
> 
> Thanks for the report.
> 
> > 
> > I didn't debug this further, if you have problems reproducing or need more
> > infos tell me. The defconfig I'm using is attached.
> > 
> 
> Does this fix it for you?
> 
> -- Steve
> 
> diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
> index 3796eb37fb12..6dbec46b7703 100644
> --- a/scripts/recordmcount.h
> +++ b/scripts/recordmcount.h
> @@ -389,11 +389,8 @@ static int nop_mcount(Elf_Shdr const *const relhdr,
>  			mcountsym = get_mcountsym(sym0, relp, str0);
>  
>  		if (mcountsym == Elf_r_sym(relp) && !is_fake_mcount(relp)) {
> -			if (make_nop) {
> +			if (make_nop)
>  				ret = make_nop((void *)ehdr, _w(shdr->sh_offset) + _w(relp->r_offset));
> -				if (ret < 0)
> -					return -1;
> -			}

Yes, this patch fixes building for me.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v4 4/8] recordmcount: Rewrite error/success handling
  2019-10-09 15:22         ` Uwe Kleine-König
@ 2019-10-10 16:23           ` Steven Rostedt
  -1 siblings, 0 replies; 20+ messages in thread
From: Steven Rostedt @ 2019-10-10 16:23 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Matt Helsley, LKML, Ingo Molnar, kernel, linux-arm-kernel

On Wed, 9 Oct 2019 17:22:18 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:

> > diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
> > index 3796eb37fb12..6dbec46b7703 100644
> > --- a/scripts/recordmcount.h
> > +++ b/scripts/recordmcount.h
> > @@ -389,11 +389,8 @@ static int nop_mcount(Elf_Shdr const *const relhdr,
> >  			mcountsym = get_mcountsym(sym0, relp, str0);
> >  
> >  		if (mcountsym == Elf_r_sym(relp) && !is_fake_mcount(relp)) {
> > -			if (make_nop) {
> > +			if (make_nop)
> >  				ret = make_nop((void *)ehdr, _w(shdr->sh_offset) + _w(relp->r_offset));
> > -				if (ret < 0)
> > -					return -1;
> > -			}  
> 
> Yes, this patch fixes building for me.

May I add to my patch:

Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Tested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

?

-- Steve

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v4 4/8] recordmcount: Rewrite error/success handling
@ 2019-10-10 16:23           ` Steven Rostedt
  0 siblings, 0 replies; 20+ messages in thread
From: Steven Rostedt @ 2019-10-10 16:23 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Ingo Molnar, linux-arm-kernel, LKML, kernel, Matt Helsley

On Wed, 9 Oct 2019 17:22:18 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:

> > diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
> > index 3796eb37fb12..6dbec46b7703 100644
> > --- a/scripts/recordmcount.h
> > +++ b/scripts/recordmcount.h
> > @@ -389,11 +389,8 @@ static int nop_mcount(Elf_Shdr const *const relhdr,
> >  			mcountsym = get_mcountsym(sym0, relp, str0);
> >  
> >  		if (mcountsym == Elf_r_sym(relp) && !is_fake_mcount(relp)) {
> > -			if (make_nop) {
> > +			if (make_nop)
> >  				ret = make_nop((void *)ehdr, _w(shdr->sh_offset) + _w(relp->r_offset));
> > -				if (ret < 0)
> > -					return -1;
> > -			}  
> 
> Yes, this patch fixes building for me.

May I add to my patch:

Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Tested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

?

-- Steve

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v4 4/8] recordmcount: Rewrite error/success handling
  2019-10-10 16:23           ` Steven Rostedt
@ 2019-10-10 20:14             ` Uwe Kleine-König
  -1 siblings, 0 replies; 20+ messages in thread
From: Uwe Kleine-König @ 2019-10-10 20:14 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Matt Helsley, LKML, Ingo Molnar, kernel, linux-arm-kernel

On Thu, Oct 10, 2019 at 12:23:21PM -0400, Steven Rostedt wrote:
> On Wed, 9 Oct 2019 17:22:18 +0200
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> 
> > > diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
> > > index 3796eb37fb12..6dbec46b7703 100644
> > > --- a/scripts/recordmcount.h
> > > +++ b/scripts/recordmcount.h
> > > @@ -389,11 +389,8 @@ static int nop_mcount(Elf_Shdr const *const relhdr,
> > >  			mcountsym = get_mcountsym(sym0, relp, str0);
> > >  
> > >  		if (mcountsym == Elf_r_sym(relp) && !is_fake_mcount(relp)) {
> > > -			if (make_nop) {
> > > +			if (make_nop)
> > >  				ret = make_nop((void *)ehdr, _w(shdr->sh_offset) + _w(relp->r_offset));
> > > -				if (ret < 0)
> > > -					return -1;
> > > -			}  
> > 
> > Yes, this patch fixes building for me.
> 
> May I add to my patch:
> 
> Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Tested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Yeah, sure.

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH v4 4/8] recordmcount: Rewrite error/success handling
@ 2019-10-10 20:14             ` Uwe Kleine-König
  0 siblings, 0 replies; 20+ messages in thread
From: Uwe Kleine-König @ 2019-10-10 20:14 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Ingo Molnar, linux-arm-kernel, LKML, kernel, Matt Helsley

On Thu, Oct 10, 2019 at 12:23:21PM -0400, Steven Rostedt wrote:
> On Wed, 9 Oct 2019 17:22:18 +0200
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> 
> > > diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
> > > index 3796eb37fb12..6dbec46b7703 100644
> > > --- a/scripts/recordmcount.h
> > > +++ b/scripts/recordmcount.h
> > > @@ -389,11 +389,8 @@ static int nop_mcount(Elf_Shdr const *const relhdr,
> > >  			mcountsym = get_mcountsym(sym0, relp, str0);
> > >  
> > >  		if (mcountsym == Elf_r_sym(relp) && !is_fake_mcount(relp)) {
> > > -			if (make_nop) {
> > > +			if (make_nop)
> > >  				ret = make_nop((void *)ehdr, _w(shdr->sh_offset) + _w(relp->r_offset));
> > > -				if (ret < 0)
> > > -					return -1;
> > > -			}  
> > 
> > Yes, this patch fixes building for me.
> 
> May I add to my patch:
> 
> Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Tested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Yeah, sure.

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 20+ messages in thread
end of thread, other threads:[~2019-10-10 20:14 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-31 18:24 [PATCH v4 0/8] recordmcount cleanups Matt Helsley
2019-07-31 18:24 ` [PATCH v4 1/8] recordmcount: Remove redundant strcmp Matt Helsley
2019-07-31 18:24 ` [PATCH v4 2/8] recordmcount: Remove uread() Matt Helsley
2019-07-31 18:24 ` [PATCH v4 3/8] recordmcount: Remove unused fd from uwrite() and ulseek() Matt Helsley
2019-07-31 18:24 ` [PATCH v4 4/8] recordmcount: Rewrite error/success handling Matt Helsley
2019-10-09 10:46   ` Uwe Kleine-König
2019-10-09 10:46     ` Uwe Kleine-König
2019-10-09 15:05     ` Steven Rostedt
2019-10-09 15:05       ` Steven Rostedt
2019-10-09 15:22       ` Uwe Kleine-König
2019-10-09 15:22         ` Uwe Kleine-König
2019-10-10 16:23         ` Steven Rostedt
2019-10-10 16:23           ` Steven Rostedt
2019-10-10 20:14           ` Uwe Kleine-König
2019-10-10 20:14             ` Uwe Kleine-König
2019-07-31 18:24 ` [PATCH v4 5/8] recordmcount: Kernel style function signature formatting Matt Helsley
2019-07-31 18:24 ` [PATCH v4 6/8] recordmcount: Kernel style formatting Matt Helsley
2019-07-31 18:24 ` [PATCH v4 7/8] recordmcount: Remove redundant cleanup() calls Matt Helsley
2019-07-31 18:24 ` [PATCH v4 8/8] recordmcount: Clarify what cleanup() does Matt Helsley
2019-08-02 17:47 ` [PATCH v4 0/8] recordmcount cleanups Steven Rostedt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.