Re: [PATCH][SPARC] Fix GNU_Sparc_HWCAPS and GNU_Sparc_HWCAPS2 tag values

Re: [PATCH][SPARC] Fix GNU_Sparc_HWCAPS and GNU_Sparc_HWCAPS2 tag values

[Mark Wielaard]

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

On Wed, 2016-06-08 at 16:29 +0200, Jose E. Marchesi wrote:
> Hi people!  Just a little patch to make readelf -A to print a nice list
> of comma-separated hwcap names, instead of rubbish :)
> 
> 2016-06-08  Jose E. Marchesi  <jose.marchesi@oracle.com>
> 
> 	* sparc_attrs.c (sparc_check_object_attribute): Fix the
> 	calculation of GNU_Sparc_HWCAPS and GNU_Sparc_HWCAPS2 tags values.

A simple testcase for inclusion into tests/run-readelf-A.sh would be
appreciated.

> diff --git a/backends/sparc_attrs.c b/backends/sparc_attrs.c
> index e95b577..505a01d 100644
> --- a/backends/sparc_attrs.c
> +++ b/backends/sparc_attrs.c
> @@ -41,33 +41,60 @@ sparc_check_object_attribute (Ebl *ebl __attribute__ ((unused)),
>  			      const char *vendor, int tag, uint64_t value,
>  			      const char **tag_name, const char **value_name)
>  {
> +  static const char *hwcaps[30] =
> +    {
> +      "mul32", "div32", "fsmuld", "v8plus", "popc", "vis", "vis2",
> +      "asi_blk_init", "fmaf", NULL, "vis3", "hpc", "random", "trans", "fjfmau",
> +      "ima", "asi_cache_sparing", "aes", "des", "kasumi", "camellia",
> +      "md5", "sha1", "sha256", "sha512", "mpmul", "mont", "pause",
> +      "cbcond", "crc32c"
> +    };
> +
> +  static const char *hwcaps2[11] =
> +    {
> +      "fjathplus", "vis3b", "adp", "sparc5", "mwait", "xmpmul",
> +      "xmont", "nsec", "fjathhpc", "fjdes", "fjaes"
> +    };
> +
> +  /* NAME must be large enough to contain up to N comma-separated list
> +     of atributes in the arrays defined above.  */
> +  static char name [30 * 16 + 30];
> +  name[0] = '\0';

This approach is not thread-safe. But if we keep the current interface
(which expects it can return a pointer to a non-mutable static string in
value_name) I don't see a good alternative. Given that it is only used
in readelf, which isn't multi-threaded, I think it is fine for now.

Given that you have made more than 10 commits now we probably should
give you commit access and appoint you sparc maintainer. Do you have a
FAS account? https://admin.fedoraproject.org/accounts

Thanks,

Mark

Re: [PATCH][SPARC] Fix GNU_Sparc_HWCAPS and GNU_Sparc_HWCAPS2 tag values

[Roland McGrath]

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

We should change the libebl interfaces whenever a new one seems better.

Re: [PATCH][SPARC] Fix GNU_Sparc_HWCAPS and GNU_Sparc_HWCAPS2 tag values

[Mark Wielaard]

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

On Thu, 2016-06-09 at 17:42 +0200, Jose E. Marchesi wrote:
>     Given that you have made more than 10 commits now we probably should
>     give you commit access and appoint you sparc maintainer. Do you have a
>     FAS account? https://admin.fedoraproject.org/accounts
> 
> Yes, I am jemarch there.

OK, I added you to Elf Utils Group (gitelfutils) and sponsored you.
If you uploaded a Public SSH Key you should now be able to pull/push
through ssh://jemarch@git.fedorahosted.org/git/elfutils.git

I see we don't have the commit rules written out, but you know how it
roughly works. Always post patches to this list according to
CONTRIBUTING. Ideally someone reviews and approves/acks within a week.
Otherwise use your own judgment whether it is something that can be
pushed after a week if there were no objections (but do write to the
list that you have done that).

I'll post a patch for CONTRIBUTING with the above written out a bit
more.

Thanks,

Mark

Re: [PATCH][SPARC] Fix GNU_Sparc_HWCAPS and GNU_Sparc_HWCAPS2 tag values

[Jose E. Marchesi]

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


    On Wed, 2016-06-08 at 16:29 +0200, Jose E. Marchesi wrote:
    > Hi people!  Just a little patch to make readelf -A to print a nice list
    > of comma-separated hwcap names, instead of rubbish :)
    > 
    > 2016-06-08  Jose E. Marchesi  <jose.marchesi@oracle.com>
    > 
    > 	* sparc_attrs.c (sparc_check_object_attribute): Fix the
    > 	calculation of GNU_Sparc_HWCAPS and GNU_Sparc_HWCAPS2 tags values.
    
    A simple testcase for inclusion into tests/run-readelf-A.sh would be
    appreciated.

I will add a test.
    
    > diff --git a/backends/sparc_attrs.c b/backends/sparc_attrs.c
    > index e95b577..505a01d 100644
    > --- a/backends/sparc_attrs.c
    > +++ b/backends/sparc_attrs.c
    > @@ -41,33 +41,60 @@ sparc_check_object_attribute (Ebl *ebl __attribute__ ((unused)),
    >  			      const char *vendor, int tag, uint64_t value,
    >  			      const char **tag_name, const char **value_name)
    >  {
    > +  static const char *hwcaps[30] =
    > +    {
    > +      "mul32", "div32", "fsmuld", "v8plus", "popc", "vis", "vis2",
    > +      "asi_blk_init", "fmaf", NULL, "vis3", "hpc", "random", "trans", "fjfmau",
    > +      "ima", "asi_cache_sparing", "aes", "des", "kasumi", "camellia",
    > +      "md5", "sha1", "sha256", "sha512", "mpmul", "mont", "pause",
    > +      "cbcond", "crc32c"
    > +    };
    > +
    > +  static const char *hwcaps2[11] =
    > +    {
    > +      "fjathplus", "vis3b", "adp", "sparc5", "mwait", "xmpmul",
    > +      "xmont", "nsec", "fjathhpc", "fjdes", "fjaes"
    > +    };
    > +
    > +  /* NAME must be large enough to contain up to N comma-separated list
    > +     of atributes in the arrays defined above.  */
    > +  static char name [30 * 16 + 30];
    > +  name[0] = '\0';
    
    This approach is not thread-safe. But if we keep the current interface
    (which expects it can return a pointer to a non-mutable static string in
    value_name) I don't see a good alternative. Given that it is only used
    in readelf, which isn't multi-threaded, I think it is fine for now.

Ok.  I will drop a note in the code so we don't forget that.
    
    Given that you have made more than 10 commits now we probably should
    give you commit access and appoint you sparc maintainer. Do you have a
    FAS account? https://admin.fedoraproject.org/accounts

Yes, I am jemarch there.

[PATCH][SPARC] Fix GNU_Sparc_HWCAPS and GNU_Sparc_HWCAPS2 tag values

[Jose E. Marchesi]

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


Hi people!  Just a little patch to make readelf -A to print a nice list
of comma-separated hwcap names, instead of rubbish :)

2016-06-08  Jose E. Marchesi  <jose.marchesi@oracle.com>

	* sparc_attrs.c (sparc_check_object_attribute): Fix the
	calculation of GNU_Sparc_HWCAPS and GNU_Sparc_HWCAPS2 tags values.


diff --git a/backends/sparc_attrs.c b/backends/sparc_attrs.c
index e95b577..505a01d 100644
--- a/backends/sparc_attrs.c
+++ b/backends/sparc_attrs.c
@@ -41,33 +41,60 @@ sparc_check_object_attribute (Ebl *ebl __attribute__ ((unused)),
 			      const char *vendor, int tag, uint64_t value,
 			      const char **tag_name, const char **value_name)
 {
+  static const char *hwcaps[30] =
+    {
+      "mul32", "div32", "fsmuld", "v8plus", "popc", "vis", "vis2",
+      "asi_blk_init", "fmaf", NULL, "vis3", "hpc", "random", "trans", "fjfmau",
+      "ima", "asi_cache_sparing", "aes", "des", "kasumi", "camellia",
+      "md5", "sha1", "sha256", "sha512", "mpmul", "mont", "pause",
+      "cbcond", "crc32c"
+    };
+
+  static const char *hwcaps2[11] =
+    {
+      "fjathplus", "vis3b", "adp", "sparc5", "mwait", "xmpmul",
+      "xmont", "nsec", "fjathhpc", "fjdes", "fjaes"
+    };
+
+  /* NAME must be large enough to contain up to N comma-separated list
+     of atributes in the arrays defined above.  */
+  static char name [30 * 16 + 30];
+  name[0] = '\0';
+
   if (!strcmp (vendor, "gnu"))
     switch (tag)
       {
       case 4:
-	*tag_name = "GNU_Sparc_HWCAPS";
-	static const char *hwcaps[30] =
-	  {
-	    "mul32", "div32", "fsmuld", "v8plus", "popc", "vis", "vis2",
-	    "asi_blk_init", "fmaf", NULL, "vis3", "hpc", "random", "trans", "fjfmau",
-	    "ima", "asi_cache_sparing", "aes", "des", "kasumi", "camellia",
-	    "md5", "sha1", "sha256", "sha512", "mpmul", "mont", "pause",
-	    "cbcond", "crc32c"
-	  };
-	if (value < 30 && hwcaps[value] != NULL)
-	  *value_name = hwcaps[value];
-	return true;
-
       case 8:
-	*tag_name = "GNU_Sparc_HWCAPS2";
-	static const char *hwcaps2[11] =
-	  {
-	    "fjathplus", "vis3b", "adp", "sparc5", "mwait", "xmpmul",
-	    "xmont", "nsec", "fjathhpc", "fjdes", "fjaes"
-	  };
-	if (value < 11)
-	  *value_name = hwcaps2[value];
-	return true;
+        {
+          const char **caps;
+          int cap;
+          
+          if (tag == 4)
+            {
+              *tag_name = "GNU_Sparc_HWCAPS";
+              caps = hwcaps;
+              cap = sizeof (hwcaps) / sizeof (hwcaps[0]);
+            }
+          else
+            {
+              *tag_name = "GNU_Sparc_HWCAPS2";
+              caps = hwcaps2;
+              cap = sizeof (hwcaps2) / sizeof (hwcaps2[0]);
+            }
+          
+          char *s = name;
+          for (cap--; cap >= 0; cap--)
+            if (value & (1 << cap))
+              {
+                if (*s != '\0')
+                  s = strcat (s, ",");
+                s = strcat (s, caps[cap]);
+              }
+          
+          *value_name = s;
+          return true;
+        }
       }
 
   return false;