linux-kernel - Re: [PATCH v5 2/7] perf annotate: Add cross arch annotate support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160819075034.GG1041@n2100.armlinux.org.uk>
Date:   Fri, 19 Aug 2016 08:50:34 +0100
From:   Russell King - ARM Linux <linux@...linux.org.uk>
To:     Ravi Bangoria <ravi.bangoria@...ux.vnet.ibm.com>
Cc:     linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
        acme@...nel.org, peterz@...radead.org, mingo@...hat.com,
        alexander.shishkin@...ux.intel.com, treeze.taeung@...il.com,
        naveen.n.rao@...ux.vnet.ibm.com, markus@...ppelsdorf.de,
        chris.ryder@....com, pawel.moll@....com, mhiramat@...nel.org,
        jolsa@...nel.org, mpe@...erman.id.au, hemant@...ux.vnet.ibm.com,
        namhyung@...nel.org
Subject: Re: [PATCH v5 2/7] perf annotate: Add cross arch annotate support

On Fri, Aug 19, 2016 at 10:59:01AM +0530, Ravi Bangoria wrote:
> -static struct ins instructions[] = {
> +static struct ins instructions_x86[] = {
>  	{ .name = "add",   .ops  = &mov_ops, },
>  	{ .name = "addl",  .ops  = &mov_ops, },
>  	{ .name = "addq",  .ops  = &mov_ops, },
>  	{ .name = "addw",  .ops  = &mov_ops, },
>  	{ .name = "and",   .ops  = &mov_ops, },
> -#ifdef __arm__
> -	{ .name = "b",     .ops  = &jump_ops, }, // might also be a call
> -	{ .name = "bcc",   .ops  = &jump_ops, },
> -	{ .name = "bcs",   .ops  = &jump_ops, },
> -	{ .name = "beq",   .ops  = &jump_ops, },
> -	{ .name = "bge",   .ops  = &jump_ops, },
> -	{ .name = "bgt",   .ops  = &jump_ops, },
> -	{ .name = "bhi",   .ops  = &jump_ops, },
> -	{ .name = "bl",    .ops  = &call_ops, },
> -	{ .name = "bls",   .ops  = &jump_ops, },
> -	{ .name = "blt",   .ops  = &jump_ops, },
> -	{ .name = "blx",   .ops  = &call_ops, },
> -	{ .name = "bne",   .ops  = &jump_ops, },
> -#endif

Notice that ARM includes a lot of other instructions from this table,
not just those above.

>  	{ .name = "bts",   .ops  = &mov_ops, },
>  	{ .name = "call",  .ops  = &call_ops, },
>  	{ .name = "callq", .ops  = &call_ops, },
> @@ -456,6 +444,21 @@ static struct ins instructions[] = {
>  	{ .name = "retq",  .ops  = &ret_ops, },
>  };
>  
> +static struct ins instructions_arm[] = {
> +	{ .name = "b",     .ops  = &jump_ops, }, /* might also be a call */
> +	{ .name = "bcc",   .ops  = &jump_ops, },
> +	{ .name = "bcs",   .ops  = &jump_ops, },
> +	{ .name = "beq",   .ops  = &jump_ops, },
> +	{ .name = "bge",   .ops  = &jump_ops, },
> +	{ .name = "bgt",   .ops  = &jump_ops, },
> +	{ .name = "bhi",   .ops  = &jump_ops, },
> +	{ .name = "bl",    .ops  = &call_ops, },
> +	{ .name = "bls",   .ops  = &jump_ops, },
> +	{ .name = "blt",   .ops  = &jump_ops, },
> +	{ .name = "blx",   .ops  = &call_ops, },
> +	{ .name = "bne",   .ops  = &jump_ops, },
> +};
> +
...
> +	if (!strcmp(norm_arch, NORM_X86)) {
> +		instructions = instructions_x86;
> +		nmemb = ARRAY_SIZE(instructions_x86);
> +	} else if (!strcmp(norm_arch, NORM_ARM)) {
> +		instructions = instructions_arm;
> +		nmemb = ARRAY_SIZE(instructions_arm);

But these changes result in _only_ the ones that were in the #if __arm__
being matched.  This is wrong.

If we want to go that way, we need to add _all_ arm instructions to
instructions_arm, not just those within the #if.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.