Avoid qname cache when possible

Author: lwaern-intel

include/simics/dmllib.h

@@ -2256,36 +2256,55 @@ __qname(dml_qname_cache_t *cache, const char *fmt, ...)
         return (const char *)s;
 }
 
-UNUSED static char *
-_DML_get_qname_alloc(_identity_t id, const _id_info_t *id_infos,
-                     const char *dev_name) {
+UNUSED static const char *
+__static_qname(_identity_t id, const _id_info_t *id_infos,
+               const char *dev_name)
+{
     _id_info_t info = id_infos[id.id - 1];
-
     const char *logname = info.logname;
 
     // In order to distinguish the device object from any other, its id_info
     // logname is always "dev", but we want its name when it comes to qname.
     if (strcmp(logname, "dev") == 0) {
-        return MM_STRDUP(dev_name);
+        return dev_name;
     }
 
+    // avoid the qname cache if we can
     if (info.dimensions == 0) {
-        return MM_STRDUP(logname);
+        return logname;
     }
 
+    return NULL;
+}
+
+UNUSED static char *
+_DML_get_qname_alloc(_identity_t id, const _id_info_t *id_infos,
+                     const char *dev_name)
+{
+    const char *simple = __static_qname(id, id_infos, dev_name);
+    if (simple != NULL) {
+            return MM_STRDUP(simple);
+    }
+
+    _id_info_t info = id_infos[id.id - 1];
     uint32 indices[info.dimensions];
     uint32 index = id.encoded_index;
     for (int32 i = info.dimensions - 1; i >= 0; --i) {
         indices[i] = index % info.dimsizes[i];
         index /= info.dimsizes[i];
     }
 
-    return _DML_format_indices(logname, indices, info.dimensions);
+    return _DML_format_indices(info.logname, indices, info.dimensions);
 }
 
 UNUSED static const char *
 _DML_get_qname(_identity_t id, const _id_info_t *id_infos,
-               dml_qname_cache_t *cache, const char *dev_name) {
+               dml_qname_cache_t *cache, const char *dev_name)
+{
+    const char *simple = __static_qname(id, id_infos, dev_name);
+    if (simple != NULL) {
+            return simple;
+    }
     char *temp_qname = _DML_get_qname_alloc(id, id_infos, dev_name);
     const char *qname = __qname(cache, "%s", temp_qname);
     MM_FREE(temp_qname);

test/1.4/lib/T_qname.dml

@@ -8,6 +8,8 @@ device test;
 
 group test;
 
+group arr[i < 10];
+
 method init() {
     // _qname() is built on the identity de/serialization machinery,
     // which takes care to distinguish between the device object
@@ -16,4 +18,31 @@ method init() {
     // string it retrieves from the identity de/serialization machinery.
     assert strcmp(dev._qname(), "test") == 0;
     assert strcmp(test._qname(), "test") == 0;
+
+    // qname depends on a weird circular caching mechanism which gives
+    // incorrect names if you have too many (currently >4) indexed qnames in
+    // flux simultaneously. A couple of hacks are applied to reduce the risk
+    // that this happens. This test checks the edges of when it breaks and when
+    // it doesn't.
+    local const char *s[5];
+    for (local int i = 0; i < s.len; i++) {
+        s[i] = arr[i].qname;
+    }
+    local strbuf_t sb = sb_newf(
+        "%s %s %s %s %s",
+        s[0], s[1], s[2], s[3], s[4]);
+    // circular buffer overflow, arr[4] overwrites arr[0]
+    assert strcmp(sb_str(&sb), "arr[4] arr[1] arr[2] arr[3] arr[4]") == 0;
+
+    // some usages of qname don't affect the cache:
+    // constant indexed
+    local const char *_ = arr[4].qname;
+    // unindexed
+    _ = test._qname();
+    // non-constant indexed qname in a log statement causes log string rewrite
+    local int i = 5;
+    log info: "%s", arr[i].qname;
+    sb_fmt(&sb, "%s %s %s %s %s", s[0], s[1], s[2], s[3], s[4]);
+    // arr[1] is unaffected by the above qname expressions
+    assert strcmp(sb_str(&sb), "arr[4] arr[1] arr[2] arr[3] arr[4]") == 0;
 }