Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 12 Jun 2020 18:05:34 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     Matt Helsley <mhelsley@vmware.com>
Cc:     linux-kernel@vger.kernel.org, Josh Poimboeuf <jpoimboe@redhat.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Sami Tolvanen <samitolvanen@google.com>,
        Julien Thierry <jthierry@redhat.com>,
        Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
Subject: Re: [RFC][PATCH v4 18/32] objtool: mcount: Move nop_mcount()
Message-ID: <20200612160534.GD2554@hirez.programming.kicks-ass.net>
References: <cover.1591125127.git.mhelsley@vmware.com>
 <7109ceb239a88c2901eeb7f52c29f69cdb413cd3.1591125127.git.mhelsley@vmware.com>
 <20200612132656.GQ2531@hirez.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200612132656.GQ2531@hirez.programming.kicks-ass.net>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Jun 12, 2020 at 03:26:57PM +0200, Peter Zijlstra wrote:
> On Tue, Jun 02, 2020 at 12:50:11PM -0700, Matt Helsley wrote:
> > +static int nop_mcount(struct section * const rels,
> > +		      const char *const txtname)
> > +{
> > +	struct reloc *reloc;
> > +	struct section *txts = find_section_by_index(lf, rels->sh.sh_info);
> > +	unsigned mcountsym = 0;
> > +	int once = 0;
> > +
> > +	list_for_each_entry(reloc, &rels->reloc_list, list) {
> > +		int ret = -1;
> > +
> > +		if (!mcountsym)
> > +			mcountsym = get_mcountsym(reloc);
> > +
> > +		if (mcountsym == GELF_R_INFO(reloc->sym->idx, reloc->type) && !is_fake_mcount(reloc)) {
> 
> This makes no sense to me; why not have mcountsym be a 'struct symbol
> *' and have get_mcountsym() return one of those.
> 
> 	if (reloc->sym == mcountsym && ... )
> 
> is much nicer, no?

On top of that, I suppose we can do something like the below.

Then you can simply write:

	if (reloc->sym->class == SYM_MCOUNT && ..)

---

diff --git a/kernel/locking/Makefile b/kernel/locking/Makefile
index 45452facff3b..94e4b8fcf9c1 100644
--- a/kernel/locking/Makefile
+++ b/kernel/locking/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 # Any varying coverage in these files is non-deterministic
 # and is generally not a function of system call inputs.
-KCOV_INSTRUMENT		:= n
+# KCOV_INSTRUMENT		:= n
 
 obj-y += mutex.o semaphore.o rwsem.o percpu-rwsem.o
 
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 432417a83902..133c0c285be6 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -341,6 +341,24 @@ static int read_sections(struct elf *elf)
 	return 0;
 }
 
+static bool is_mcount_symbol(const char *name)
+{
+	if (name[0] == '.')
+		name++;
+
+	if (name[0] == '_')
+		name++;
+
+	return !strcmp(name, "mcount", 6) ||
+	       !strcmp(name, "_fentry__") ||
+	       !strcmp(name, "_gnu_mcount_nc");
+}
+
+static bool is_kcov_symbol(const char *name)
+{
+	return !strncmp(name, "__sanitizer_cov_", 16);
+}
+
 static int read_symbols(struct elf *elf)
 {
 	struct section *symtab, *symtab_shndx, *sec;
@@ -410,6 +428,12 @@ static int read_symbols(struct elf *elf)
 		} else
 			sym->sec = find_section_by_index(elf, 0);
 
+
+		if (is_mcount_symbol(sym->name))
+			sym->class = SYM_MCOUNT;
+		else if (is_kcov_symbol(sym->name))
+			sym->class = SYM_KCOV;
+
 		sym->offset = sym->sym.st_value;
 		sym->len = sym->sym.st_size;
 
diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h
index 78a2db23b8b6..3c1cccb7b5ff 100644
--- a/tools/objtool/elf.h
+++ b/tools/objtool/elf.h
@@ -42,6 +42,12 @@ struct section {
 	bool changed, text, rodata, noinstr;
 };
 
+enum symbol_class {
+	SYM_REGULAR = 0,
+	SYM_MCOUNT,
+	SYM_KCOV,
+};
+
 struct symbol {
 	struct list_head list;
 	struct rb_node node;
@@ -55,6 +61,7 @@ struct symbol {
 	unsigned long offset;
 	unsigned int len;
 	struct symbol *pfunc, *cfunc, *alias;
+	enum symbol_class class;
 	bool uaccess_safe;
 };