From c4cecd9d1933011c6cda663a2550cd78b2e0519b Mon Sep 17 00:00:00 2001 From: Gurchetan Singh Date: Thu, 2 Apr 2026 16:45:50 -0700 Subject: [PATCH] gfxstream: cereal: fix 'None' in gfxstream codegen Commit e27e41a842d4 ("vulkan,spirv: update headers") exposed a flaw in the cerealgenerator. It modified -- among other things -- the VkDeviceCreateInfo struct in vk.xml. In the update, the len="enabledLayerCount,null-terminated" attribute was removed from the ppEnabledLayerNames member. The gfxstream code generator processes ppEnabledLayerNames (which is a const char* const*), it identifies it as an "array of strings". However, because the len attribute is now missing, vulkanType.getLengthExpression() returns None. This leads to errors like: gfxstream_guest_vk_autogen_impl/gen/goldfish_vk_counting_guest.cpp:642:30: error: use of undeclared identifier 'None' 642 | for (uint32_t i = 0; i < None; ++i) | ^~~~ 1 error generated. This patch adds various length access checks to prevent this from happening. TEST=m vulkan.ranchu Reviewed-by: David Gilhooley Part-of: --- .../codegen/scripts/cereal/counting.py | 28 +++++---- .../codegen/scripts/cereal/deepcopy.py | 17 +++--- .../codegen/scripts/cereal/handlemap.py | 9 +-- .../codegen/scripts/cereal/marshaling.py | 10 +++- .../scripts/cereal/reservedmarshaling.py | 38 ++++++------ .../codegen/scripts/cereal/testing.py | 60 ++++++++++--------- 6 files changed, 87 insertions(+), 75 deletions(-) diff --git a/src/gfxstream/codegen/scripts/cereal/counting.py b/src/gfxstream/codegen/scripts/cereal/counting.py index 2738f0d8bdd..ef313473e34 100644 --- a/src/gfxstream/codegen/scripts/cereal/counting.py +++ b/src/gfxstream/codegen/scripts/cereal/counting.py @@ -370,25 +370,27 @@ class VulkanCountingCodegen(VulkanTypeIterator): lenAccessGuard = self.lenAccessorGuard(vulkanType) self.genCount("sizeof(uint32_t)") - if lenAccessGuard is not None: - self.cgen.beginIf(lenAccessGuard) - self.cgen.beginFor("uint32_t i = 0", "i < %s" % lenAccess, "++i") - self.genCount("sizeof(uint32_t) + (%s[i] ? strlen(%s[i]) : 0)" % (access, access)) - self.cgen.endFor() - if lenAccessGuard is not None: - self.cgen.endIf() + if lenAccess is not None: + if lenAccessGuard is not None: + self.cgen.beginIf(lenAccessGuard) + self.cgen.beginFor("uint32_t i = 0", "i < %s" % lenAccess, "++i") + self.genCount("sizeof(uint32_t) + (%s[i] ? strlen(%s[i]) : 0)" % (access, access)) + self.cgen.endFor() + if lenAccessGuard is not None: + self.cgen.endIf() def onStaticArr(self, vulkanType): access = self.exprValueAccessor(vulkanType) lenAccess = self.lenAccessor(vulkanType) lenAccessGuard = self.lenAccessorGuard(vulkanType) - if lenAccessGuard is not None: - self.cgen.beginIf(lenAccessGuard) - finalLenExpr = "%s * %s" % (lenAccess, self.cgen.sizeofExpr(vulkanType)) - if lenAccessGuard is not None: - self.cgen.endIf() - self.genCount(finalLenExpr) + if lenAccess is not None: + if lenAccessGuard is not None: + self.cgen.beginIf(lenAccessGuard) + finalLenExpr = "%s * %s" % (lenAccess, self.cgen.sizeofExpr(vulkanType)) + if lenAccessGuard is not None: + self.cgen.endIf() + self.genCount(finalLenExpr) def onStructExtension(self, vulkanType): sTypeParam = copy(vulkanType) diff --git a/src/gfxstream/codegen/scripts/cereal/deepcopy.py b/src/gfxstream/codegen/scripts/cereal/deepcopy.py index ac6483434ea..392c4c255bf 100644 --- a/src/gfxstream/codegen/scripts/cereal/deepcopy.py +++ b/src/gfxstream/codegen/scripts/cereal/deepcopy.py @@ -155,16 +155,17 @@ class DeepcopyCodegen(VulkanTypeIterator): lenAccessLhs = self.lenAccessorLhs(vulkanType) self.cgen.stmt("%s = nullptr" % accessRhs) - self.cgen.beginIf("%s && %s" % (accessLhs, lenAccessLhs)) + if lenAccessLhs is not None: + self.cgen.beginIf("%s && %s" % (accessLhs, lenAccessLhs)) - self.cgen.stmt( \ - "%s = %s->strDupArray(%s, %s)" % \ - (accessRhs, - self.poolVarName, - accessLhs, - lenAccessLhs)) + self.cgen.stmt( \ + "%s = %s->strDupArray(%s, %s)" % \ + (accessRhs, + self.poolVarName, + accessLhs, + lenAccessLhs)) - self.cgen.endIf() + self.cgen.endIf() def onStaticArr(self, vulkanType): accessLhs = self.exprAccessorValueLhs(vulkanType) diff --git a/src/gfxstream/codegen/scripts/cereal/handlemap.py b/src/gfxstream/codegen/scripts/cereal/handlemap.py index 8ccc43cdfd4..a18880450cf 100644 --- a/src/gfxstream/codegen/scripts/cereal/handlemap.py +++ b/src/gfxstream/codegen/scripts/cereal/handlemap.py @@ -108,10 +108,11 @@ class HandleMapCodegen(VulkanTypeIterator): accessLhs = self.exprAccessor(vulkanType) lenAccess = self.lenAccessor(vulkanType) - self.cgen.stmt("%s->mapHandles_%s(%s%s, %s)" % \ - (self.handlemapVarName, vulkanType.typeName, - self.makeCastExpr(vulkanType.getForAddressAccess().getForNonConstAccess()), - accessLhs, lenAccess)) + if lenAccess is not None: + self.cgen.stmt("%s->mapHandles_%s(%s%s, %s)" % \ + (self.handlemapVarName, vulkanType.typeName, + self.makeCastExpr(vulkanType.getForAddressAccess().getForNonConstAccess()), + accessLhs, lenAccess)) def onStructExtension(self, vulkanType): access = self.exprAccessor(vulkanType) diff --git a/src/gfxstream/codegen/scripts/cereal/marshaling.py b/src/gfxstream/codegen/scripts/cereal/marshaling.py index 01a41bbe9ee..a140916ad0c 100644 --- a/src/gfxstream/codegen/scripts/cereal/marshaling.py +++ b/src/gfxstream/codegen/scripts/cereal/marshaling.py @@ -455,8 +455,9 @@ class VulkanMarshalingCodegen(VulkanTypeIterator): lenAccess = self.lenAccessor(vulkanType) if self.direction == "write": - self.cgen.stmt("saveStringArray(%s, %s, %s)" % (self.streamVarName, - access, lenAccess)) + if lenAccess is not None: + self.cgen.stmt("saveStringArray(%s, %s, %s)" % (self.streamVarName, + access, lenAccess)) else: castExpr = \ self.makeCastExpr( \ @@ -468,7 +469,10 @@ class VulkanMarshalingCodegen(VulkanTypeIterator): def onStaticArr(self, vulkanType): access = self.exprValueAccessor(vulkanType) lenAccess = self.lenAccessor(vulkanType) - finalLenExpr = "%s * %s" % (lenAccess, self.cgen.sizeofExpr(vulkanType)) + if lenAccess is not None: + finalLenExpr = "%s * %s" % (lenAccess, self.cgen.sizeofExpr(vulkanType)) + else: + finalLenExpr = self.cgen.sizeofExpr(vulkanType) self.genStreamCall(vulkanType, access, finalLenExpr) # Old version VkEncoder may have some sType values conflict with VkDecoder diff --git a/src/gfxstream/codegen/scripts/cereal/reservedmarshaling.py b/src/gfxstream/codegen/scripts/cereal/reservedmarshaling.py index bfa36cea03a..9b675937ced 100644 --- a/src/gfxstream/codegen/scripts/cereal/reservedmarshaling.py +++ b/src/gfxstream/codegen/scripts/cereal/reservedmarshaling.py @@ -548,25 +548,26 @@ class VulkanReservedMarshalingCodegen(VulkanTypeIterator): lenAccessGuard = self.lenAccessorGuard(vulkanType) if self.direction == "write": - self.cgen.beginBlock() + if lenAccess is not None: + self.cgen.beginBlock() - self.cgen.stmt("uint32_t c = 0") - if lenAccessGuard is not None: - self.cgen.beginIf(lenAccessGuard) - self.cgen.stmt("c = %s" % (lenAccess)) - if lenAccessGuard is not None: + self.cgen.stmt("uint32_t c = 0") + if lenAccessGuard is not None: + self.cgen.beginIf(lenAccessGuard) + self.cgen.stmt("c = %s" % (lenAccess)) + if lenAccessGuard is not None: + self.cgen.endIf() + self.genMemcpyAndIncr(self.ptrVar, "(uint32_t*)" ,"&c", "sizeof(uint32_t)", toBe = True, actualSize = 4) + + self.cgen.beginFor("uint32_t i = 0", "i < c", "++i") + self.cgen.stmt("uint32_t l = %s ? strlen(%s[i]): 0" % (access, access)) + self.genMemcpyAndIncr(self.ptrVar, "(uint32_t*)" ,"&l", "sizeof(uint32_t)", toBe = True, actualSize = 4) + self.cgen.beginIf("l") + self.genMemcpyAndIncr(self.ptrVar, "(char*)", "(%s[i])" % access, "l") self.cgen.endIf() - self.genMemcpyAndIncr(self.ptrVar, "(uint32_t*)" ,"&c", "sizeof(uint32_t)", toBe = True, actualSize = 4) + self.cgen.endFor() - self.cgen.beginFor("uint32_t i = 0", "i < c", "++i") - self.cgen.stmt("uint32_t l = %s ? strlen(%s[i]): 0" % (access, access)) - self.genMemcpyAndIncr(self.ptrVar, "(uint32_t*)" ,"&l", "sizeof(uint32_t)", toBe = True, actualSize = 4) - self.cgen.beginIf("l") - self.genMemcpyAndIncr(self.ptrVar, "(char*)", "(%s[i])" % access, "l") - self.cgen.endIf() - self.cgen.endFor() - - self.cgen.endBlock() + self.cgen.endBlock() else: castExpr = \ self.makeCastExpr( \ @@ -578,8 +579,9 @@ class VulkanReservedMarshalingCodegen(VulkanTypeIterator): def onStaticArr(self, vulkanType): access = self.exprValueAccessor(vulkanType) lenAccess = self.lenAccessor(vulkanType) - finalLenExpr = "%s * %s" % (lenAccess, self.cgen.sizeofExpr(vulkanType)) - self.genStreamCall(vulkanType, access, finalLenExpr) + if lenAccess is not None: + finalLenExpr = "%s * %s" % (lenAccess, self.cgen.sizeofExpr(vulkanType)) + self.genStreamCall(vulkanType, access, finalLenExpr) # Old version VkEncoder may have some sType values conflict with VkDecoder # of new versions. For host decoder, it should not carry the incorrect old diff --git a/src/gfxstream/codegen/scripts/cereal/testing.py b/src/gfxstream/codegen/scripts/cereal/testing.py index fdf4b1d7f9b..68925aa762d 100644 --- a/src/gfxstream/codegen/scripts/cereal/testing.py +++ b/src/gfxstream/codegen/scripts/cereal/testing.py @@ -205,53 +205,55 @@ class VulkanEqualityCodegen(VulkanTypeIterator): vulkanType, "Mismatch in string array pointer nullness") - equalLenExpr = self.makeEqualExpr(lenAccessLhs, lenAccessRhs) + if lenAccessLhs is not None and lenAccessRhs is not None: + equalLenExpr = self.makeEqualExpr(lenAccessLhs, lenAccessRhs) - self.compareWithConsequence( \ - equalLenExpr, - vulkanType, "Lengths not equal in string array") + self.compareWithConsequence( \ + equalLenExpr, + vulkanType, "Lengths not equal in string array") - self.compareWithConsequence( \ - equalLenExpr, - vulkanType, "Lengths not equal in string array") + self.compareWithConsequence( \ + equalLenExpr, + vulkanType, "Lengths not equal in string array") - self.cgen.beginIf("%s && %s" % (equalLenExpr, bothNotNullExpr)) + self.cgen.beginIf("%s && %s" % (equalLenExpr, bothNotNullExpr)) - loopVar = "i" - accessLhs = "*(%s + %s)" % (accessLhs, loopVar) - accessRhs = "*(%s + %s)" % (accessRhs, loopVar) - forInit = "uint32_t %s = 0" % loopVar - forCond = "%s < (uint32_t)%s" % (loopVar, lenAccessLhs) - forIncr = "++%s" % loopVar + loopVar = "i" + accessLhs = "*(%s + %s)" % (accessLhs, loopVar) + accessRhs = "*(%s + %s)" % (accessRhs, loopVar) + forInit = "uint32_t %s = 0" % loopVar + forCond = "%s < (uint32_t)%s" % (loopVar, lenAccessLhs) + forIncr = "++%s" % loopVar - if lenAccessGuardLhs is not None: - self.cgen.beginIf(lenAccessGuardLhs) + if lenAccessGuardLhs is not None: + self.cgen.beginIf(lenAccessGuardLhs) - self.cgen.beginFor(forInit, forCond, forIncr) + self.cgen.beginFor(forInit, forCond, forIncr) - self.compareWithConsequence( - self.makeEqualStringExpr(accessLhs, accessRhs), - vulkanType, "Unequal string in string array") + self.compareWithConsequence( + self.makeEqualStringExpr(accessLhs, accessRhs), + vulkanType, "Unequal string in string array") - self.cgen.endFor() + self.cgen.endFor() + + if lenAccessGuardLhs is not None: + self.cgen.endIf() - if lenAccessGuardLhs is not None: self.cgen.endIf() - self.cgen.endIf() - def onStaticArr(self, vulkanType): accessLhs = self.exprAccessorLhs(vulkanType) accessRhs = self.exprAccessorRhs(vulkanType) lenAccessLhs = self.lenAccessorLhs(vulkanType) - finalLenExpr = "%s * %s" % (lenAccessLhs, - self.cgen.sizeofExpr(vulkanType)) + if lenAccessLhs is not None: + finalLenExpr = "%s * %s" % (lenAccessLhs, + self.cgen.sizeofExpr(vulkanType)) - self.compareWithConsequence( - self.makeEqualBufExpr(accessLhs, accessRhs, finalLenExpr), - vulkanType, "Unequal static array") + self.compareWithConsequence( + self.makeEqualBufExpr(accessLhs, accessRhs, finalLenExpr), + vulkanType, "Unequal static array") def onStructExtension(self, vulkanType): lhs = self.exprAccessorLhs(vulkanType)