Skip to content
Snippets Groups Projects
  • Jakub Jelinek's avatar
    0bb27b81
    libgomp: Fix up GOMP_task on s390x · 0bb27b81
    Jakub Jelinek authored
    On Wed, Jan 20, 2021 at 05:04:39PM +0100, Florian Weimer wrote:
    > Sorry, this appears to cause OpenMP task state corruption in RPM.  We
    > have only seen this on s390x.
    
    Haven't actually verified it, but my suspection is that this is a caller
    stack corruption.
    
    We play with fire with the GOMP_task API/ABI extensions, the GOMP_task
    function used to be:
    void
    GOMP_task (void (*fn) (void *), void *data, void (*cpyfn) (void *, void *),
               long arg_size, long arg_align, bool if_clause, unsigned flags);
    and later:
    void
    GOMP_task (void (*fn) (void *), void *data, void (*cpyfn) (void *, void *),
               long arg_size, long arg_align, bool if_clause, unsigned flags,
               void **depend);
    and later:
    void
    GOMP_task (void (*fn) (void *), void *data, void (*cpyfn) (void *, void *),
               long arg_size, long arg_align, bool if_clause, unsigned flags,
               void **depend, int priority);
    and now:
    void
    GOMP_task (void (*fn) (void *), void *data, void (*cpyfn) (void *, void *),
               long arg_size, long arg_align, bool if_clause, unsigned flags,
               void **depend, int priority, void *detach)
    and which of those depend, priority and detach argument is present depends
    on the bits in flags.
    I'm afraid the compiler just decided to spill the detach = NULL store in
      if ((flags & GOMP_TASK_FLAG_DETACH) == 0)
        detach = NULL;
    on s390x into the argument stack slot.  Not a problem if the caller passes
    all those 10 arguments, but if not, can clobber random stack location.
    
    This hack should fix it up.  Priority doesn't need changing, but I've
    changed it anyway just to be safe.  With the patch none of the 3 arguments
    are ever modified, so I'd hope gcc doesn't decide to spill something
    unrelated there.
    
    2021-01-20  Jakub Jelinek  <jakub@redhat.com>
    
    	* task.c (GOMP_task): Rename priority argument to priority_arg,
    	add priority automatic variable and modify that variable.  Instead of
    	clearing detach argument when GOMP_TASK_FLAG_DETACH bit is not set,
    	check flags for that bit.
    0bb27b81
    History
    libgomp: Fix up GOMP_task on s390x
    Jakub Jelinek authored
    On Wed, Jan 20, 2021 at 05:04:39PM +0100, Florian Weimer wrote:
    > Sorry, this appears to cause OpenMP task state corruption in RPM.  We
    > have only seen this on s390x.
    
    Haven't actually verified it, but my suspection is that this is a caller
    stack corruption.
    
    We play with fire with the GOMP_task API/ABI extensions, the GOMP_task
    function used to be:
    void
    GOMP_task (void (*fn) (void *), void *data, void (*cpyfn) (void *, void *),
               long arg_size, long arg_align, bool if_clause, unsigned flags);
    and later:
    void
    GOMP_task (void (*fn) (void *), void *data, void (*cpyfn) (void *, void *),
               long arg_size, long arg_align, bool if_clause, unsigned flags,
               void **depend);
    and later:
    void
    GOMP_task (void (*fn) (void *), void *data, void (*cpyfn) (void *, void *),
               long arg_size, long arg_align, bool if_clause, unsigned flags,
               void **depend, int priority);
    and now:
    void
    GOMP_task (void (*fn) (void *), void *data, void (*cpyfn) (void *, void *),
               long arg_size, long arg_align, bool if_clause, unsigned flags,
               void **depend, int priority, void *detach)
    and which of those depend, priority and detach argument is present depends
    on the bits in flags.
    I'm afraid the compiler just decided to spill the detach = NULL store in
      if ((flags & GOMP_TASK_FLAG_DETACH) == 0)
        detach = NULL;
    on s390x into the argument stack slot.  Not a problem if the caller passes
    all those 10 arguments, but if not, can clobber random stack location.
    
    This hack should fix it up.  Priority doesn't need changing, but I've
    changed it anyway just to be safe.  With the patch none of the 3 arguments
    are ever modified, so I'd hope gcc doesn't decide to spill something
    unrelated there.
    
    2021-01-20  Jakub Jelinek  <jakub@redhat.com>
    
    	* task.c (GOMP_task): Rename priority argument to priority_arg,
    	add priority automatic variable and modify that variable.  Instead of
    	clearing detach argument when GOMP_TASK_FLAG_DETACH bit is not set,
    	check flags for that bit.