[Beignet] *** SPAM LEVEL 5.929 *** beignet: libEGL ABI abuse (getting at symbols not intended to be public)

Zhigang Gong zhigang.gong at linux.intel.com
Fri Jul 5 02:29:45 PDT 2013


Thanks for the test result. The attached is v2 patch to fix
the crash when run it without X server. Could you help to
test it again?

And Simon, could you also give this patch a try? Thanks.

On Fri, Jul 05, 2013 at 05:15:34PM +0800, He Junyan wrote:
> Hi Zhigang:
> 
> I think this problem casued by the X server process.
> On my platform, I prefer not to run X by default, and there is no
> Xorg process.  This may cause the EGL crash problem.
> When Xorg is running, all the cases are successful including
> compiler_fill_gl_image.
> 
> 
> On 07/05/2013 04:37 PM, He Junyan wrote:
> >Hi zhigang:
> >
> >This patch seems have some problem to me.
> >The unit test will crash every time,
> >
> >Program received signal SIGSEGV, Segmentation fault.
> >cl_gbm_set_image_extension (gbm=0x618640, display=0x0) at
> >/home/robinhe/CL/beignet/src/x11/gbm_dri2_x11_platform.c:99
> >99        struct _hack_dri2_egl_display *dri2_dpy = (struct
> >_hack_dri2_egl_display*)egl_dpy->DriverData;
> >(gdb) bt
> >#0  cl_gbm_set_image_extension (gbm=0x618640, display=0x0) at
> >/home/robinhe/CL/beignet/src/x11/gbm_dri2_x11_platform.c:99
> >#1  0x00007ffff6e34689 in intel_driver_open (intel=0x615e50,
> >props=0x7fffffffd0d0) at
> >/home/robinhe/CL/beignet/src/intel/intel_driver.c:216
> >#2  0x00007ffff6e34879 in cl_intel_driver_new
> >(props=0x7fffffffd0d0) at
> >/home/robinhe/CL/beignet/src/intel/intel_driver.c:378
> >#3  0x00007ffff6e30cb0 in cl_context_new (props=0x7fffffffd0d0) at
> >/home/robinhe/CL/beignet/src/cl_context.c:145
> >#4  0x00007ffff6e30e92 in cl_create_context (properties=0x618600,
> >num_devices=<optimized out>, devices=0x7ffff7dd9d80,
> >pfn_notify=<optimized out>, user_data=<optimized out>,
> >    errcode_ret=0x7fffffffd14c) at
> >/home/robinhe/CL/beignet/src/cl_context.c:116
> >#5  0x00007ffff6e299f1 in clCreateContext (properties=0x618600,
> >num_devices=1, devices=0x7ffff7dd9d80, pfn_notify=0,
> >user_data=<optimized out>, errcode_ret=0x7fffffffdab4)
> >    at /home/robinhe/CL/beignet/src/cl_api.c:203
> >#6  0x00007ffff7bbd811 in cl_ocl_init () at
> >/home/robinhe/CL/beignet/utests/utest_helper.cpp:358
> >#7  0x0000000000401172 in main (argc=1, argv=0x7fffffffe438) at
> >/home/robinhe/CL/beignet/utests/utest_run.cpp:33
> >(gdb)
> >
> >It seems the display struct may be changed between our mesa version.
> >
> >
> >On 07/05/2013 04:16 PM, Zhigang Gong wrote:
> >>Sorry, I just found I sent the wrong patch in my last email. Please
> >>ignore that patch and use this one:
> >>0001-CLGL-Refine-the-hack-of-gbm-extension-initialization.patch.
> >>
> >>On Thu, Jul 04, 2013 at 08:26:09PM +0800, Zhigang Gong wrote:
> >>>On Wed, Jul 03, 2013 at 01:35:08PM +0200, Simon Richter wrote:
> >>>>Hi,
> >>>>
> >>>>On 03.07.2013 12:01, Zhigang Gong wrote:
> >>>>
> >>>>>The gbm device doesn't init lookup_image method and the user data. The lookup_image is only
> >>>>>initialized when use the gbm device to create an egl display and then initialize the egl drm
> >>>>>platform which is not our use model.
> >>>>My plan for Beignet in Debian is to upload the releases to the regular
> >>>>track where they can move into the next release, and direct git
> >>>>snapshots at the experimental track (I've made an exception with 0.1,
> >>>>because that version breaks installed OpenCL software with the error
> >>>>returns from the query APIs), so ideally I'd like to have a solution
> >>>>before that.
> >>>>
> >>>>Can this be delegated to the Mesa project by means of a change request
> >>>>in their BTS?
> >>>I will do that when I have time, may be within this month. Before that,
> >>>I have a workaround patch to get the gl texture sharing work. Could you
> >>>help to test it? It's still very hacky but avoid reference to the mesa's
> >>>internal symbol directly.
> >>>
> >>>>Current state:
> >>>>
> >>>>  - Debian#712880 (autogenerated dependencies too weak) blocks Mesa 9
> >>>>from propagating along the regular package track (unstable -> testing ->
> >>>>stable). I expect that to be fixed soon.
> >>>>
> >>>>  - Debian#712903 (dependency on a non-public symbol) blocks Beignet from
> >>>>propagating within Debian
> >>>>
> >>>>  - Debian#630344 (support for private symbols in package dependency
> >>>>calculations) is looming above our heads. When implemented, this feature
> >>>>will make builds of packages using private symbols from other packages fail.
> >>>>
> >>>>What I can do is drop EGL support in the Debian packages, at least for
> >>>>the regular track, which would allow us to be part of the release, but
> >>>>at reduced functionality.
> >>>>
> >>>>    Simon
> >>>>
> >>>>Bug references:
> >>>>
> >>>>http://bugs.debian.org/712880
> >>>>http://bugs.debian.org/712903
> >>>>http://buge.debian.org/630344
> >>>>
> >>>
> >>>>_______________________________________________
> >>>>Beignet mailing list
> >>>>Beignet at lists.freedesktop.org
> >>>>http://lists.freedesktop.org/mailman/listinfo/beignet
> >>> From cacf883a0fed9e56beb943edb1f9e43ed87c928b Mon Sep 17 00:00:00 2001
> >>>From: Zhigang Gong<zhigang.gong at linux.intel.com>
> >>>Date: Thu, 4 Jul 2013 19:12:52 +0800
> >>>Subject: [PATCH] GBE: Clear the value map when start a new scalarize pass.
> >>>
> >>>The scalarize pass is a function pass, and the valueMap should
> >>>be a per-function data rather than a per-unit data. The reason
> >>>we put it in the unit data structure is that the scalarize pass
> >>>is before the GenWriter pass thus there is no ir::Function exists.
> >>>
> >>>As there may be multiple kernel functions in one unit, if we don't
> >>>clear the valueMap each time running a new scalarize pass, the previous
> >>>data may cause some unexpected behaviour. For example, the previous
> >>>instructions have been already erased, then latter a new instruction
> >>>in this function may be created in the same position of the erased
> >>>instruction, then it breaks this valueMap. That's the root cause why
> >>>we run the unit test several times and may encounter an assertion
> >>>sometime.
> >>>
> >>>This commit also modify the ir::unit layer implementation to remove
> >>>the dependency of llvm from that layer. In general, we should not add
> >>>llvm related code to the ir layer.
> >>>
> >>>Signed-off-by: Zhigang Gong<zhigang.gong at linux.intel.com>
> >>>---
> >>>  backend/src/ir/unit.cpp               |   14 --------------
> >>>  backend/src/ir/unit.hpp               |   20 +++++++-------------
> >>>  backend/src/llvm/llvm_gen_backend.cpp |    7 ++++---
> >>>  backend/src/llvm/llvm_scalarize.cpp   |    2 +-
> >>>  4 files changed, 12 insertions(+), 31 deletions(-)
> >>>
> >>>diff --git a/backend/src/ir/unit.cpp b/backend/src/ir/unit.cpp
> >>>index 01e1eb1..4aeffe9 100644
> >>>--- a/backend/src/ir/unit.cpp
> >>>+++ b/backend/src/ir/unit.cpp
> >>>@@ -21,12 +21,6 @@
> >>>   * \file unit.cpp
> >>>   * \author Benjamin Segovia<benjamin.segovia at intel.com>
> >>>   */
> >>>-#include "llvm/Config/config.h"
> >>>-#if LLVM_VERSION_MINOR <= 2
> >>>-#include "llvm/Instructions.h"
> >>>-#else
> >>>-#include "llvm/IR/Instructions.h"
> >>>-#endif  /* LLVM_VERSION_MINOR <= 2 */
> >>>  #include "ir/unit.hpp"
> >>>  #include "ir/function.hpp"
> >>>@@ -59,14 +53,6 @@ namespace ir {
> >>>      constantSet.append(data, name, size, alignment);
> >>>    }
> >>>-  void Unit::removeDeadValues()
> >>>-  {
> >>>-    for(auto &it : valueMap) {
> >>>-      llvm::Instruction* I = llvm::dyn_cast<llvm::Instruction>(it.first.first);  //fake value
> >>>-      if((I == NULL) || (I->getParent() == NULL))
> >>>-        valueMap.erase(it.first);
> >>>-    }
> >>>-  }
> >>>    std::ostream &operator<< (std::ostream &out, const Unit &unit) {
> >>>      unit.apply([&out] (const Function &fn) { out << fn << std::endl; });
> >>>      return out;
> >>>diff --git a/backend/src/ir/unit.hpp b/backend/src/ir/unit.hpp
> >>>index 1017f5f..9e3d66a 100644
> >>>--- a/backend/src/ir/unit.hpp
> >>>+++ b/backend/src/ir/unit.hpp
> >>>@@ -24,13 +24,6 @@
> >>>  #ifndef __GBE_IR_UNIT_HPP__
> >>>  #define __GBE_IR_UNIT_HPP__
> >>>-#include "llvm/Config/config.h"
> >>>-#if LLVM_VERSION_MINOR <= 2
> >>>-#include "llvm/Value.h"
> >>>-#else
> >>>-#include "llvm/IR/Value.h"
> >>>-#endif  /* LLVM_VERSION_MINOR <= 2 */
> >>>-
> >>>  #include "ir/constant.hpp"
> >>>  #include "ir/register.hpp"
> >>>  #include "sys/hash_map.hpp"
> >>>@@ -49,7 +42,7 @@ namespace ir {
> >>>    {
> >>>    public:
> >>>      typedef hash_map<std::string, Function*> FunctionSet;
> >>>-    typedef std::pair<llvm::Value*, uint32_t> ValueIndex;
> >>>+    typedef std::pair<void*, uint32_t> ValueIndex;
> >>>      /*! Create an empty unit */
> >>>      Unit(PointerSize pointerSize = POINTER_32_BITS);
> >>>      /*! Release everything (*including* the function pointers) */
> >>>@@ -84,8 +77,8 @@ namespace ir {
> >>>      /*! Some values will not be allocated. For example a vector extract and
> >>>       * a vector insertion when scalarize the vector load/store
> >>>       */
> >>>-    void newValueProxy(llvm::Value *real,
> >>>-                       llvm::Value *fake,
> >>>+    void newValueProxy(void *real,
> >>>+                       void *fake,
> >>>                         uint32_t realIndex = 0u,
> >>>                         uint32_t fakeIndex = 0u) {
> >>>        const ValueIndex key(fake, fakeIndex);
> >>>@@ -93,10 +86,11 @@ namespace ir {
> >>>        GBE_ASSERT(valueMap.find(key) == valueMap.end()); // Do not insert twice
> >>>        valueMap[key] = value;
> >>>      }
> >>>-    /* remove fake values that removed by other pass */
> >>>-    void removeDeadValues(void);
> >>>+
> >>>+    void clearValueMap() { valueMap.clear(); }
> >>>+
> >>>      /*! Return the value map */
> >>>-    const map<ValueIndex, ValueIndex>& getValueMap(void) const { return valueMap; }
> >>>+    const map<ValueIndex, ValueIndex> &getValueMap(void) const { return valueMap; }
> >>>    private:
> >>>      friend class ContextInterface; //!< Can free modify the unit
> >>>      hash_map<std::string, Function*> functions; //!< All the defined functions
> >>>diff --git a/backend/src/llvm/llvm_gen_backend.cpp b/backend/src/llvm/llvm_gen_backend.cpp
> >>>index 40f2667..36d8129 100644
> >>>--- a/backend/src/llvm/llvm_gen_backend.cpp
> >>>+++ b/backend/src/llvm/llvm_gen_backend.cpp
> >>>@@ -307,8 +307,10 @@ namespace gbe
> >>>      }
> >>>      /*! After scalarize pass, there are some valueMap in unit,
> >>>       *  use this function to copy from unit valueMap */
> >>>-    void initValueMap(const map<ValueIndex, ValueIndex>& vMap) {
> >>>-      valueMap.insert(vMap.begin(), vMap.end());
> >>>+    void initValueMap(const map<ir::Unit::ValueIndex, ir::Unit::ValueIndex> &vMap) {
> >>>+      for(auto &it : vMap)
> >>>+        newValueProxy((Value*)it.second.first, (Value*)it.first.first,
> >>>+                      it.second.second, it.first.second);
> >>>      }
> >>>      /*! Mostly used for the preallocated registers (lids, gids) */
> >>>      void newScalarProxy(ir::Register reg, Value *value, uint32_t index = 0u) {
> >>>@@ -1207,7 +1209,6 @@ namespace gbe
> >>>      }
> >>>      ctx.startFunction(F.getName());
> >>>-    unit.removeDeadValues();
> >>>      this->regTranslator.clear();
> >>>      this->regTranslator.initValueMap(unit.getValueMap());
> >>>      this->labelMap.clear();
> >>>diff --git a/backend/src/llvm/llvm_scalarize.cpp b/backend/src/llvm/llvm_scalarize.cpp
> >>>index bab2236..41674b6 100644
> >>>--- a/backend/src/llvm/llvm_scalarize.cpp
> >>>+++ b/backend/src/llvm/llvm_scalarize.cpp
> >>>@@ -773,7 +773,7 @@ namespace gbe {
> >>>      intTy = IntegerType::get(module->getContext(), 32);
> >>>      floatTy = Type::getFloatTy(module->getContext());
> >>>      builder = new IRBuilder<>(module->getContext());
> >>>-    unit.removeDeadValues();
> >>>+    unit.clearValueMap();
> >>>      scalarizeArgs(F);
> >>>      typedef ReversePostOrderTraversal<Function*> RPOTType;
> >>>-- 
> >>>1.7.9.5
> >>>
> >>>_______________________________________________
> >>>Beignet mailing list
> >>>Beignet at lists.freedesktop.org
> >>>http://lists.freedesktop.org/mailman/listinfo/beignet
> >>
> >>
> >>_______________________________________________
> >>Beignet mailing list
> >>Beignet at lists.freedesktop.org
> >>http://lists.freedesktop.org/mailman/listinfo/beignet
> >
> >
> >
> >_______________________________________________
> >Beignet mailing list
> >Beignet at lists.freedesktop.org
> >http://lists.freedesktop.org/mailman/listinfo/beignet
> 

> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-CLGL-Refine-the-hack-of-gbm-extension-initialization.patch
Type: text/x-diff
Size: 7924 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/beignet/attachments/20130705/cacb48e3/attachment.patch>


More information about the Beignet mailing list