This is an archive of the discontinued LLVM Phabricator instance.

Use safe yaml load_all
ClosedPublic

Authored by kongyi on Oct 19 2021, 7:21 AM.

Diff Detail

Event Timeline

kongyi requested review of this revision.Oct 19 2021, 7:21 AM
kongyi created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptOct 19 2021, 7:21 AM

LGTM, I wonder what triggered that change?

This revision is now accepted and ready to land.Oct 20 2021, 10:29 PM

LGTM, I wonder what triggered that change?

This was discovered by our internal security audit.

This revision was automatically updated to reflect the committed changes.
shchenz added a subscriber: shchenz.Nov 1 2021, 1:14 AM

Hi @kongyi , this breaks opt-viewer.py usage on the AIX platform.

opt-viewer/opt-viewer.py  test.opt.yaml --demangler cat
For faster parsing, you may want to install libYAML for PyYAML
Reading YAML files...
        1 of 1multiprocessing.pool.RemoteTraceback: 
"""
Traceback (most recent call last):
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 121, in worker
    result = (True, func(*args, **kwds))
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 44, in mapstar
    return list(map(*args))
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optpmap.py", line 25, in _wrapped_func
    return func(argument, filter_)
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optrecord.py", line 277, in get_remarks
    docs = yaml.safe_load_all(f, Loader=Loader)
TypeError: safe_load_all() got an unexpected keyword argument 'Loader'
"""
kongyi added a comment.Nov 1 2021, 1:36 AM

Hi @kongyi , this breaks opt-viewer.py usage on the AIX platform.

opt-viewer/opt-viewer.py  test.opt.yaml --demangler cat
For faster parsing, you may want to install libYAML for PyYAML
Reading YAML files...
        1 of 1multiprocessing.pool.RemoteTraceback: 
"""
Traceback (most recent call last):
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 121, in worker
    result = (True, func(*args, **kwds))
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 44, in mapstar
    return list(map(*args))
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optpmap.py", line 25, in _wrapped_func
    return func(argument, filter_)
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optrecord.py", line 277, in get_remarks
    docs = yaml.safe_load_all(f, Loader=Loader)
TypeError: safe_load_all() got an unexpected keyword argument 'Loader'
"""

Can you try removing the second argument (Loader=Loader) and check if it works?

Hi @kongyi , this breaks opt-viewer.py usage on the AIX platform.

opt-viewer/opt-viewer.py  test.opt.yaml --demangler cat
For faster parsing, you may want to install libYAML for PyYAML
Reading YAML files...
        1 of 1multiprocessing.pool.RemoteTraceback: 
"""
Traceback (most recent call last):
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 121, in worker
    result = (True, func(*args, **kwds))
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 44, in mapstar
    return list(map(*args))
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optpmap.py", line 25, in _wrapped_func
    return func(argument, filter_)
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optrecord.py", line 277, in get_remarks
    docs = yaml.safe_load_all(f, Loader=Loader)
TypeError: safe_load_all() got an unexpected keyword argument 'Loader'
"""

Can you try removing the second argument (Loader=Loader) and check if it works?

No, still fails.

Traceback (most recent call last):
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 121, in worker
    result = (True, func(*args, **kwds))
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 44, in mapstar
    return list(map(*args))
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optpmap.py", line 25, in _wrapped_func
    return func(argument, filter_)
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optrecord.py", line 282, in get_remarks
    for remark in docs:
  File "/home/czhengsz/test/optviewer/optviewer_env/lib64/python3.7/site-packages/yaml/__init__.py", line 93, in load_all
    yield loader.get_data()
  File "/home/czhengsz/test/optviewer/optviewer_env/lib64/python3.7/site-packages/yaml/constructor.py", line 45, in get_data
    return self.construct_document(self.get_node())
  File "/home/czhengsz/test/optviewer/optviewer_env/lib64/python3.7/site-packages/yaml/constructor.py", line 55, in construct_document
    data = self.construct_object(node)
  File "/home/czhengsz/test/optviewer/optviewer_env/lib64/python3.7/site-packages/yaml/constructor.py", line 100, in construct_object
    data = constructor(self, node)
  File "/home/czhengsz/test/optviewer/optviewer_env/lib64/python3.7/site-packages/yaml/constructor.py", line 429, in construct_undefined
    node.start_mark)
yaml.constructor.ConstructorError: could not determine a constructor for the tag '!Missed'
kongyi added a comment.Nov 1 2021, 2:14 AM

Hi @kongyi , this breaks opt-viewer.py usage on the AIX platform.

opt-viewer/opt-viewer.py  test.opt.yaml --demangler cat
For faster parsing, you may want to install libYAML for PyYAML
Reading YAML files...
        1 of 1multiprocessing.pool.RemoteTraceback: 
"""
Traceback (most recent call last):
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 121, in worker
    result = (True, func(*args, **kwds))
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 44, in mapstar
    return list(map(*args))
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optpmap.py", line 25, in _wrapped_func
    return func(argument, filter_)
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optrecord.py", line 277, in get_remarks
    docs = yaml.safe_load_all(f, Loader=Loader)
TypeError: safe_load_all() got an unexpected keyword argument 'Loader'
"""

Can you try removing the second argument (Loader=Loader) and check if it works?

No, still fails.

Traceback (most recent call last):
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 121, in worker
    result = (True, func(*args, **kwds))
  File "/opt/freeware/lib64/python3.7/multiprocessing/pool.py", line 44, in mapstar
    return list(map(*args))
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optpmap.py", line 25, in _wrapped_func
    return func(argument, filter_)
  File "/home/czhengsz/llvm/dev/llvm-project/llvm/tools/opt-viewer/optrecord.py", line 282, in get_remarks
    for remark in docs:
  File "/home/czhengsz/test/optviewer/optviewer_env/lib64/python3.7/site-packages/yaml/__init__.py", line 93, in load_all
    yield loader.get_data()
  File "/home/czhengsz/test/optviewer/optviewer_env/lib64/python3.7/site-packages/yaml/constructor.py", line 45, in get_data
    return self.construct_document(self.get_node())
  File "/home/czhengsz/test/optviewer/optviewer_env/lib64/python3.7/site-packages/yaml/constructor.py", line 55, in construct_document
    data = self.construct_object(node)
  File "/home/czhengsz/test/optviewer/optviewer_env/lib64/python3.7/site-packages/yaml/constructor.py", line 100, in construct_object
    data = constructor(self, node)
  File "/home/czhengsz/test/optviewer/optviewer_env/lib64/python3.7/site-packages/yaml/constructor.py", line 429, in construct_undefined
    node.start_mark)
yaml.constructor.ConstructorError: could not determine a constructor for the tag '!Missed'

Okay, I'll revert this change for now.

thanks, when you have a new fix, I am glad to verify it
on AIX before you commit.

qiucf added a subscriber: qiucf.Nov 4 2021, 2:31 AM

The YAML test files may contain some tags, here including !Passed !Missed !Failure !AnalysisAliasing !AnalysisFPCommute !Analysis.

I think it's the reason that the safe loader can't find appropriate constructor for these tags.

--- a/llvm/tools/opt-viewer/optrecord.py
+++ b/llvm/tools/opt-viewer/optrecord.py
@@ -235,6 +235,7 @@ class Remark(yaml.YAMLObject):


 class Analysis(Remark):
+    yaml_loader = yaml.SafeLoader
     yaml_tag = '!Analysis'

     @property
@@ -243,14 +244,17 @@ class Analysis(Remark):


 class AnalysisFPCommute(Analysis):
+    yaml_loader = yaml.SafeLoader
     yaml_tag = '!AnalysisFPCommute'


 class AnalysisAliasing(Analysis):
+    yaml_loader = yaml.SafeLoader
     yaml_tag = '!AnalysisAliasing'


 class Passed(Remark):
+    yaml_loader = yaml.SafeLoader
     yaml_tag = '!Passed'

     @property
@@ -259,6 +263,7 @@ class Passed(Remark):


 class Missed(Remark):
+    yaml_loader = yaml.SafeLoader
     yaml_tag = '!Missed'

     @property
@@ -266,6 +271,7 @@ class Missed(Remark):
         return "red"

 class Failure(Missed):
+    yaml_loader = yaml.SafeLoader
     yaml_tag = '!Failure'

 def get_remarks(input_file, filter_=None):
@@ -274,7 +280,7 @@ def get_remarks(input_file, filter_=None):
     file_remarks = defaultdict(functools.partial(defaultdict, list))

     with io.open(input_file, encoding = 'utf-8') as f:
-        docs = yaml.load_all(f, Loader=Loader)
+        docs = yaml.safe_load_all(f)

         filter_e = None
         if filter_:

And I think this isn't related to AIX. I found the same issue on Linux. Try llvm/test/tools/opt-viewer/Inputs/suppress/s.opt.yaml.